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

Ptl add adaptive timestep #25

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

peterlafollette
Copy link
Contributor

@peterlafollette peterlafollette commented May 9, 2024

Adaptive time step was added and tested for stability.

Additions

-The model can now run with an adaptive time step. The adaptive time step can be turned on or off in the config file. If it is not specified in the config file, it defaults to false. There are three different internal time steps: 1/12 hrs, 1/6 hrs, and 1 hr. The time step is 1 hr when precip and ponded head are 0, which saves a lot of runtime in arid areas where large periods of time will not have precip. The time step is 1/6 hrs when the precip is less than or equal to 1 cm/h and there is no ponded head. The time step is 1/12 hrs (the value we have been using for the vast majority of LGAR's development) when ponded head is greater than 0, or when precip is greater than 1 cm/h.

Removals

Changes

-Updated README in configs to include adaptive_timestep.

-If the model uses a fixed time step, then the GIUH ordinates and queue are unaffected (the model time step and resolution of the GIUH ordinates and queue will be the same). However, when the model uses an adaptive time step, the GIUH ordinates and queue are initialized with the smallest internal time step, which is 5 minutes. Input for the GIUH convolution integral is then always resampled to match its resolution. Therefore, the model time step is adaptive, but the resolution of the GIUH queue / ordinates are fixed. An alternative approach I developed but decided against was to just compute the GIUH at the hourly resolution, as the model outputs hourly data.

-Discovered a bug where theta is very close to theta_r, and convergence might be possible, but the function lgar_theta_mass_balance doesn't converge after a few minutes. The bug comes from the fact that the condition (fabs(delta_mass - delta_mass_prev) < 1E-15) doesn't trigger; the change in mass is indeed small, but it is 1E-11 rather than 1E-15 and changes very slowly, just because of the sensitivity of the theta-psi relationship for some parameter sets in particular. Rather than changing the tolerance to be less strict, I added a new check where the loop will break for extremely large psi values, which have limited physical meaning in the first place.

-Changed the psi value at which calc_Se_from_h returns 1.0 to be 1E-10 rather than 1E-1, making the function accurate for small psi values. There are some cases where we need Se values that are very close to but slightly less than 1, for psi values that are less than 1E-1.

Testing

  1. Stability testing with 40 k parameter sets successful (adaptive and fixed time steps).
  2. Phillipsburg, Bushland, synthetic, and unit tests give expected outputs.
  3. Integrated tests on GitHub.
  4. Tested in ngen, including 3 layer tests, 1 layer tests, and running multiple catchments from the same realization file, including a mix of adaptive and fixed time step config files.

Screenshots

Notes

Todos

Checklist

  • PR has an informative and human-readable title
  • Changes are limited to a single goal (no scope creep)
  • Code can be automatically merged (no conflicts)
  • Code follows project standards (link if applicable)
  • Passes all existing automated tests
  • Any change in functionality is tested
  • New functions are documented (with a description, list of inputs, and expected output)
  • Placeholder code is flagged / future todos are captured in comments
  • Visually tested in supported browsers and devices (see checklist below 👇)
  • Project documentation has been updated (including the "Unreleased" section of the CHANGELOG)
  • Reviewers requested with the Reviewers tool ➡️

Testing checklist

Target Environment support

  • Windows
  • Linux
  • Browser

Accessibility

  • Keyboard friendly
  • Screen reader friendly

Other

  • Is useable without CSS
  • Is useable without JS
  • Flexible from small to large screens
  • No linting errors or warnings
  • JavaScript tests are passing

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

1 participant