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 time precision issue for long VIC runs #668

Merged
merged 5 commits into from
Jan 19, 2017

Conversation

yixinmao
Copy link
Contributor

Before this fix, the timestamps of long VIC runs were incorrect in some cases. This PR is in response to issue #663.

Reason: The reason of this issue is that during the step where timestamps are made (in make_dmy), the length of a timestep is first converted into the specified time unit, denoted as dt_time_units; then a series of timestamps for the whole running period is determined by incrementally increase the timestamp by dt_time_units. When dt_time_units is not precise, the incremented timestamp will accumulate the small error of dt_time_units, potentially resulting in incorrect timestamp after a long simulation time.

Example: for example, say the time unit is day, and the model timestep is hourly. In this case, dt_time_units = 1/24, which is stored as a finite number in the computer with a very small cut-off error. The accumulated error due to this results in an incorrect timestamp shift after ~14 years, and eventually terminates model outputs afeter ~31 years.

Solution: when generating the timestamp for each timestep, calculate the accumulating time length from the simulating start time precisely, then store the time. In this way, there is only one cut-off error ofr all timestamps without error accumulation.

@yixinmao
Copy link
Contributor Author

@tbohn , could you take a look at this PR? If you are able to use the new code to test on your data that originally caused error, that'd be great. Thanks!

@bartnijssen
Copy link
Member

@jhamman : Could you take a quick look at this on travis? The build is failing because of an HDF library issue (is this the same problem someone else mentioned in the past week - it vaguely rings a bell).

@jhamman
Copy link
Member

jhamman commented Jan 18, 2017

The failing tests are all due to an issue with Anaconda, not VIC. I'll look into why this is happening. @yixinmao - did the test suite pass locally?

@tbohn
Copy link
Contributor

tbohn commented Jan 18, 2017

@yixinmao sure, I'm testing it out now.

@jhamman
Copy link
Member

jhamman commented Jan 18, 2017

@yixinmao - can we get a what's new note?

@yixinmao
Copy link
Contributor Author

@jhamman I'm running the tests locally now (I haven't run the test locally for a while so ran into some issues that are probably not due to this PR...). Do you mean a release note?

@jhamman
Copy link
Member

jhamman commented Jan 18, 2017

yes, a release note.

@tbohn
Copy link
Contributor

tbohn commented Jan 18, 2017

@yixinmao The code works correctly for me now. Thanks!

@bartnijssen
Copy link
Member

bartnijssen commented Jan 18, 2017

Can be merged after travis build passes (issue #669)

@jhamman jhamman added this to the 5.0.1 (Next bugfix release) milestone Jan 19, 2017
@jhamman jhamman merged commit 312b6f4 into UW-Hydro:hotfix/5.0.1 Jan 19, 2017
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.

4 participants