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

Gridded lake restart fixes #205

Closed
wants to merge 2 commits into from
Closed

Conversation

aubreyd
Copy link
Collaborator

@aubreyd aubreyd commented Oct 16, 2018

Fix for perfect restart issue in gridded routing configuration with lakes active. Now tracking lake inflow from previous timestep in restarts so that the levelpool routine has same starting conditions as through run. New qlakei variable added to hydro restarts, so will not bit-for-bit match previous restarts. No apparent issues running new code with old restarts.

Also confirmed the following:

  1. No answer changes in NWM CONUS, Croton reach-based (no lakes), and Croton gridded w/o lakes configurations.
  2. Expected answer changes in Croton gridded w/ lakes configuration.
  3. Perfect restarts in Croton gridded w/ lakes configuration.
  4. Blessed by Yates!

Aubrey Dugger added 2 commits October 16, 2018 10:39
…estep inflow in LEVELPOOL calc. Fixes perfect restart in gridded routing with lakes config.
@kafitzgerald
Copy link
Contributor

Yay, so glad to see this!! Should we also quickly update the NEWS.md file in the base directory so that we've got a log of changes for the release notes? If you're swamped I can just do a PR / push to your branch, but I'm guessing you can describe this much better than I can ;)

@kafitzgerald
Copy link
Contributor

Do we know why channel_rt : Head is different in the NWM perfect restart test? @jmills-ncar @aubreyd

The other testing failures make sense to me (new variable in the restarts).

@aubreyd
Copy link
Collaborator Author

aubreyd commented Oct 16, 2018

Is that a new failure? In my local test I saw diffs in the velocity field in the outputs, but perfect match on restart files for NWM. I have seen that velocity diff before but was ignoring since restarts matched.

@tjmills
Copy link

tjmills commented Oct 16, 2018

The perfect restart tests test outputs as well as restart files.

I have not seen the perfect restart test fail for the Head variable in channel_rt for NWM config. I honestly didn't even know that variable was in there (not in ops)

There is also still a fail for lake_inflort in gridded hydro restart, did oyu see that?

@aubreyd
Copy link
Collaborator Author

aubreyd commented Oct 16, 2018

lake_inflort wasn't the restart issue. That is an accumulated variable, so should match exactly iff you set RSTRT_SWC = 0.

@tjmills
Copy link

tjmills commented Oct 16, 2018

What does setting RSTRT_SWC = 0 do? Wondering if its better to just ignore that variable, or toggle that option

@aubreyd
Copy link
Collaborator Author

aubreyd commented Oct 16, 2018

It controls whether or not accumulated variables should be reset to 0. You must be dealing with this already since qstrmvolrt is also an accumulated variable.

@tjmills
Copy link

tjmills commented Oct 16, 2018

yeah we ignore a number of accumulated variables. @jmccreight do you have a preference here? Add lake_onflort to ignored vaeriables or set RSTRT_SWC=0?

@laurareads
Copy link
Contributor

Could be a good time to revisit why lake_inflort is an accumulated variable in the first place. I am not sure it needs to be.

@tjmills
Copy link

tjmills commented Oct 16, 2018

The HEAD fail in nwm_ana is not new behavior with this PR, it is a older problem. Because the restarts were failing for so long this slipped through. I am going back through the PRs to find where it was introduced

@tjmills
Copy link

tjmills commented Oct 16, 2018

I suspect this will trace back to the introduction of longer timescale testing, which is probably when it reared its HEAD, pun intended

@tjmills
Copy link

tjmills commented Oct 18, 2018

NOTE the changes in this PR have been incorporated into PR #211

@tjmills tjmills closed this Oct 18, 2018
tjmills pushed a commit that referenced this pull request Oct 19, 2018
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