Add ensemble functionality - #17
Conversation
|
Scientific question 1: Which values to use for alpha and beta? |
|
Implementation question 1: Do we want warning or failure if running deterministically (only one ensemble member) with noise? |
|
Implementation question 2: Do we want the number of ensemble members and noise parameters to be CLI arguments rather than environemnt variables? I.e. parameters called when running sunflow: Current decision: CLI argument |
|
Ideas for ensemble spread variables: Quantiles, 10/90 percentile |
|
Mean vs. median - which is more useful? |
|
@irenelivia If you want, you can try this out to see if anything obvious stands out as ideas for improvements or things that should be changed. Ideas for simple commands to test:
|
There was a problem hiding this comment.
Awesome work @KristianHMoller .
Three improvements suggested to strengthen this PR:
(1) Little validation bug in main.py:378 - the clearsky completeness check should include the t=0 timestamp (all_clearsky_time_steps instead of previous_day_time_steps), preventing crashes when t=0 data is missing.
(2) Clarify docstring in forecast.py:89-90 to document that return shape varies by ens_members (single-member: 3D, multi-member: 4D), eliminating confusion for future users.
(3) Add dimension check in data_io.py to validate that ensemble array dimensions match the configured ens_members count, catching programmer errors early if validation logic changes.
I tested:
- sunflow --ensemble_members 10 --time 2026-07-17T09:15Z --full_ensemble
Works seamlessly with the ensemble dimension. Nice! Very interesting to skim through the different noise images.
(Note: this still has the "arctan artifacts")
- sunflow --ensemble_members 10 --time 2026-07-17T09:15Z
Also works fine, just a small comment to change the output names of the statistical variables, so the statistic comes first in the name (easier to see on ncview for oldies like me 😄)
(Note: this still has the "arctan artifacts")
- export ENSEMBLE_STATISTICS=mean,median
sunflow --ensemble_members 10 --time 2026-07-17T09:15Z
Also works fine, outputting only mean and median statistics.
(Note: this still has the "arctan artifacts")
- sunflow --ensemble_members 0 --time 2026-07-17T09:15Z
Works fine, outputs 1 ensemble member, no arctan artifacts
- sunflow --ensemble_members 1 --time 2026-07-17T09:15Z
Works fine, outputs 1 ensemble member, no arctan artifacts
|
Mean and percentiles do not seem to cover full domain, while median does. Investigate why! EDIT: Bug solved with: dac8168 |
|
Significant speed-up should be possible by doing cropping to EDIT: Implemented with: dac8168 |
This reverts commit 5406030.
|
Thanks a lot for thorough review and testing @irenelivia This should be ready for re-review :) |
irenelivia
left a comment
There was a problem hiding this comment.
I re-tested the updated pr17 using ENSEMBLE_STATISTICS="median,p10,p90" uv run sunflow --run_mode files --ensemble_members 5 --time 2026-08-06T09:30Z
. Works well, and very nice now with the statistics showing up first in the name! See my old-school ncview:
minor comments now from a quick re-review with Claude. Can be merged afterwards!
The underlying probabilistic advection method has the option of running an ensemble with noise. However, until now, this option has not been implemented into
sunflow.This PR allows for running as an ensemble by specifying the CLI argument
--ensemble_membersto be different from one. Per default, the output will be selected ensemble statistics, but specifying--full_ensemblewhen calling sunflow will instead provide the full ensemble as output. Theoutput_modeglobal attribute in the NetCDF file will specify the type of output: full ensemble, ensemble_statistic or deterministic.If only a subset of the ensemble statistics are desired, this can be specified by exporting the environment variable
ENSEMBLE_STATISTICSwith the desired variables comma separated.The default values of the noise parameters alpha and beta are 0 for deterministic mode and using other values will result in a warning.
For ensemble mode, currently, the default values are taken from Carpentieri et al.: https://www.sciencedirect.com/science/article/pii/S030626192301139X