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

Fix issues #181 and #191 #192

Merged
merged 3 commits into from
Jan 13, 2022
Merged

Fix issues #181 and #191 #192

merged 3 commits into from
Jan 13, 2022

Conversation

verseve
Copy link
Member

@verseve verseve commented Jan 10, 2022

Fixing issues #191 and #181.

Issue #181 is fixed by checking if the NetCDF variable var has dimension time in the ncread function. If this is the case, it is a cyclic parameter that is handled by the update_cyclic! function.

cyclic parameter now also works for a static model parameter (initialized with ncread).
@visr
Copy link
Member

visr commented Jan 13, 2022

The #191 fix looks good.

For the #181 fix, I see you check if time is in the dimnames. The read_dims function is made such that size 1 dimensions are allowed, so this would break size 1 time variables. Hence the error in #181 says "Only extra dimensions of length 1 are supported."

How does return nothing work for ncread? I'd think that this would crash later during initialization. One alternative approach would be to just read in the first or current timestep, which would then be updated by update_cyclic!. Perhaps we can make use of get_at to do this.

cyclic parameters have a time dimension > 1, this is not allowed in read_dims, solution is to read the first timestep in ncread and later this is updated by the update_cyclic function.
@verseve
Copy link
Member Author

verseve commented Jan 13, 2022

Yes, good point about the #181 fix. The approach is now to include dim time=1 in dim_sel of the ncread function (ce2fe91).

@verseve verseve merged commit b1f113e into master Jan 13, 2022
@visr visr deleted the fix-issues branch January 13, 2022 14:55
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.

fixed timestep river kinematic wave Cyclic parameter: should also work for static parameter
2 participants