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

Create "convert static to state" function #49

Closed
xaviernogueira opened this issue Nov 6, 2023 · 2 comments · Fixed by #51
Closed

Create "convert static to state" function #49

xaviernogueira opened this issue Nov 6, 2023 · 2 comments · Fixed by #51
Assignees
Labels
refactor-core For core functionality, not model specifc

Comments

@xaviernogueira
Copy link
Contributor

xaviernogueira commented Nov 6, 2023

Result of a call about linking with Clearwater-riverine w/ @sjordan29 and @jrutyna 11/6/2023.

Issue:

  • Some static variables, despite not needing to be altered between timesteps in the context of Clearwater-modules alone may need to be updated between timesteps when linked with other models.
  • For computational and memory efficiency, I keep any static variable 2-dimensional (as in no time axis), this is better than replicating N timestep worth of the same grid in our main xarray.Dataset. That said, it makes it awkward to allow updates between timesteps, as it could result in time axes of different lengths IF we wanted to preserve some efficiency, or it forces us to give up on that efficiency entirely resulting in a significant memory penalty.
  • By design, only state variables can be updated between timesteps.

Proposed Solution: Upon initialization of a module, we can add a new __init__ argument convert_static_to_state where names of static variables can optionally be provided via a list. Any static variable in the list will be re-registered as a state variable, therefore it will be given a time axis, and be allowed to update between timesteps. Doing it this way allows us to only convert the variables we need to interact with, preserving efficiency where we can.

URGENT PRIORITY so Sarah and Jason can proceed.

Thoughts @aufdenkampe

@xaviernogueira xaviernogueira added the refactor-core For core functionality, not model specifc label Nov 6, 2023
@xaviernogueira xaviernogueira self-assigned this Nov 6, 2023
@aufdenkampe
Copy link
Member

@xaviernogueira, this seems like a reasonable approach. Thanks for detailing the background and proposing a good solution.

@xaviernogueira
Copy link
Contributor Author

Final solution

Converting the static variables we need to update to state variables was somewhat problematic as then the program attempts to calculate the static variables using non-existent process functions. Instead there is now an optional updateable_static_variables argument for Model.__init__. Some other small changes make the described desired behavior possible, without updating the variable type as a whole to "state".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor-core For core functionality, not model specifc
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants