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.

Sign up for GitHub

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

Jump to bottom

Fix time cyclic input and lake rating curve (column index) #241

Merged
merged 4 commits into from
Jan 12, 2023
Merged

Fix time cyclic input and lake rating curve (column index) #241

merged 4 commits into from
Jan 12, 2023

Conversation

verseve
Copy link
Contributor

@verseve verseve commented Jan 12, 2023

No description provided.

vers-w added 3 commits January 12, 2023 12:58
@vers-w
cyclic input
6cf78f7 
Time is right labeled (Clock and forcing), for cyclic input (monthly and daily) this is not the case. To correct for this an offset of 1 day was used, this should be the model time step.
@vers-w
fix column index
175b27e 
Daily lake rating curves, based on HBV-96 functionality, are provided for julian days (leap days are not counted), per CSV column. For the column index the dayofyear was used, with a maximum of 365. This has been fixed, where February 28 is day 59 and March 1 (and February 29 for leap years) is day 60.
@vers-w
update changelog
c4d7096
@verseve verseve added the bug Something isn't working label Jan 12, 2023
@verseve verseve linked an issue Jan 12, 2023 that may be closed by this pull request
Day of year for lake model #239
Closed
@verseve verseve requested a review from visr January 12, 2023 15:17
visr
visr approved these changes Jan 12, 2023
View reviewed changes
Copy link
Member

@visr visr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Would be good to add a few unit tests to make sure julian_day keeps workin as intended.

julia> julian_day(DateTime(2000,2,28))
59

julia> julian_day(DateTime(2000,2,29))
60

julia> julian_day(DateTime(2000,3,1))
60

@verseve
Copy link
Contributor Author

verseve commented Jan 12, 2023

Looks good to me. Would be good to add a few unit tests to make sure julian_day keeps workin as intended.

Thanks for the quick review! Good suggestion, I did add a few units tests.

@verseve verseve merged commit c64222a into master Jan 12, 2023
@verseve verseve deleted the time_and_lake branch January 18, 2023 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@visr visr visr approved these changes

Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Day of year for lake model
3 participants
@verseve @visr @vers-w