Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Starttime #262

Merged
merged 6 commits into from
May 30, 2023
Merged

Starttime #262

merged 6 commits into from
May 30, 2023

Conversation

verseve
Copy link
Member

@verseve verseve commented May 17, 2023

The starttime was defined one model timestep Δt ahead of the actual model time (the initial conditions timestamp (state time)). As a consequence this was also the case for the current model time. To allow for an easier interpretation of Wflow time handling, either through BMI or directly, the starttime is now equal to the state time, resulting in current model times without an offset.

The model starttime was defined 1 timestep ahead of the actual current model time, e.g.: after model initialization (Δt = 86400.0 s) with a model state time of 2000-01-01, the current model time was set at 2000-01-02. And during a simulation the current model time stayed 1 timestep ahead. This has changed: the starttime (also in the TOML file) is the model state (cold or warm) time, and the current model time reflects now the actual current model time. This time handling is also easier to interpret for users running a wflow model via BMI.
by removing subtraction of model timestep Δt from the first NetCDF time, for a standalone Wflow run this is also fine.
With clock advancement, loading forcing, updating model and optional writing of model output (removed from model `update` function). This function is also called from BMI.
@verseve verseve requested a review from JoostBuitink May 17, 2023 14:04
@JoostBuitink
Copy link
Contributor

Thanks for this work! I am (once again ;) ) a little bit confused about the handling of time, see the figure below. Here I ran the same model, but the timeseries are shifted by 1 timestep. I don't think this the expected behavior right? In this case the starttime is inferred from the forcing.nc file, which has 2008-01-02 as the first timestep.

If the first forcing timestep is set to 2008-01-02, I would expect that the first output of Wflow to also be at 2008-01-02 (as the timestep 2008-01-01 are represented by the instates). When specifying a starttime in the toml, I think the current behavior is as expected, as the first used forcing timestep is starttime + Δt. Do you agree with this, or is it too confusing?

image

@verseve
Copy link
Member Author

verseve commented May 30, 2023

Thanks for this work! I am (once again ;) ) a little bit confused about the handling of time, see the figure below. Here I ran the same model, but the timeseries are shifted by 1 timestep. I don't think this the expected behavior right? In this case the starttime is inferred from the forcing.nc file, which has 2008-01-02 as the first timestep.

Yes, when the starttime is inferred from the forcing.nc file, the starttime is set to the first timestep of the forcing.nc file. This deviates from the previous implementation. I agree it can be a bit confusing and will change it so Wflow also includes the first forcing timestep.

Change `starttime` from forcing.nc: first timestep in forcing.nc minus 1 model timestep, so the first forcing timestep is included in the Wflow run. For a  `fews_run` the `starttime` is equal to the first forcing timestep (first forcing timestep is excluded).
@verseve verseve merged commit e3de2bc into master May 30, 2023
12 checks passed
@visr visr deleted the starttime branch May 30, 2023 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants