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

Add structure(s) to iterate over all state variables #282

Open
billsacks opened this issue Feb 13, 2018 · 7 comments
Open

Add structure(s) to iterate over all state variables #282

billsacks opened this issue Feb 13, 2018 · 7 comments
Labels
code health improving internal code structure to make easier to maintain (sustainability) priority: low Background task that doesn't need to be done right away. size: large Large project that will take a few weeks or more

Comments

@billsacks
Copy link
Member

There are a number of times when we need to iterate over all state variables (i.e., variables with some persistent memory from one timestep to another), or some large subset of them. This includes:

  • restart

  • adjustments for transient PFT weights

  • adjustments for transient column / landunit weights

  • adjustments for fire

As well as some possible future needs:

  • Ability to expand / contract memory for dynamic landunits? (rather than pre-allocating everything we need up front)

  • Dynamic load balancing?

Code related to these things would be more maintainable if we had the ability to iterate over all state variables, or the subset of state variables that met some criteria.

@billsacks billsacks added the enhancement new capability or improved behavior of existing capability label Feb 13, 2018
@billsacks
Copy link
Member Author

I envision the definition of "state variable" being tied to whether it's needed on the restart file. A nice benefit of this definition is that we have automated tests (ERS, etc.) that make sure that all variables that should be on the restart file truly are on the restart file.

Thus, before we tackle this project, it's probably worth doing some initial cleanup:

  • Remove variables from the restart file that really don't need to be there

  • Ideally, also reorder some code so that fewer variables need to be on the restart file: try to make it so that variables are computed in the time step before they are used, so that auxiliary variables don't need to persist on the restart file just due to illogical ordering in the driver. (This may require a cleaner separation of the calculation of fluxes from their application: fluxes should be computed early in the time step, and then applied to state variables later in that time step - as opposed to saving fluxes until the next time step and applying them early in the next time step.)

@billsacks
Copy link
Member Author

Note to self: See the following evernote entries (sorry that this isn't useful to others; maybe I'll incorporate some relevant notes from these Evernote documents into this issue at some point):

- "Making state updates more maintainable" (evernote:///view/1172113975/s44/12e24a30-a72b-49a1-a0bd-2a9c47a067c5/12e24a30-a72b-49a1-a0bd-2a9c47a067c5/)

- "CLM redesign ideas" (evernote:///view/1172113975/s44/13c5d9e4-9cfc-4493-a9f2-b04c7eea4305/13c5d9e4-9cfc-4493-a9f2-b04c7eea4305/) (February 11, 2016 notes)

- "Handling CLM's restart variables more like history variables" (evernote:///view/1172113975/s44/fea9c49a-c6bf-4b43-8ac5-d1e13ad06b46/fea9c49a-c6bf-4b43-8ac5-d1e13ad06b46/)

@martynpclark
Copy link

@billsacks -- this is very useful. I think it is desirable to (1) be more precise about the variable type (state variable, diagnostic variable, flux, parameter, etc.) and (2) have the capability to loop over all variable types. This is useful for fluxes, for example, when there is a need to calculate the time-step average from multiple flux calls. In short, I think that the need to loop over state variables extends to other variable types also.

@billsacks billsacks added the priority: low Background task that doesn't need to be done right away. label Jan 16, 2019
mariuslam pushed a commit to NordicESMhub/ctsm that referenced this issue Aug 26, 2019
@billsacks billsacks added code health improving internal code structure to make easier to maintain (sustainability) and removed enhancement new capability or improved behavior of existing capability labels Feb 20, 2020
@billsacks
Copy link
Member Author

I started thinking about this more when looking back at code like this for harvest:

hrv_leafc_to_litter(p) = leafc(p) * m
hrv_frootc_to_litter(p) = frootc(p) * m
hrv_livestemc_to_litter(p) = livestemc(p) * m
wood_harvestc(p) = deadstemc(p) * m
hrv_livecrootc_to_litter(p) = livecrootc(p) * m
hrv_deadcrootc_to_litter(p) = deadcrootc(p) * m
hrv_xsmrpool_to_atm(p) = xsmrpool(p) * m

and similar code in a module on the TRENDY branch, dynGrossUnrepMod.F90.

An important goal in software design is that different aspects should be orthogonal, so that if you want to change one aspect of the code, you only need to make the change in one place. With our current code structure, adding a new state variable often requires many changes, including to code like the above that is doing some operation on all state variables of a certain type. This makes it much more difficult and error-prone to change the model. I feel it would help to have the ability to loop through state variables, perhaps separated on a few basic characteristics - e.g., letting you loop through all soil BGC variables, all vegetation BGC variables, etc. There are a lot of details that would need to be worked out, like how to associate a flux with a given state variable, but I feel that something like this could be very helpful long-term as the model gets more and more complex.

@wwieder
Copy link
Contributor

wwieder commented Feb 20, 2020

I'm 100% behind this, if you can make it work @billsacks !

@billsacks
Copy link
Member Author

Thanks @wwieder . This is an ambitious issue, so I don't expect to tackle it any time soon....

@billsacks billsacks added the size: large Large project that will take a few weeks or more label May 22, 2020
@billsacks
Copy link
Member Author

I was randomly giving this issue a bit of thought recently. A few thoughts:

  • As noted above, we could tie this in with a list of restart variables, since those should capture everything that is important at the start or end of a time step.

  • Specifically related to "Ability to expand / contract memory for dynamic landunits":

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code health improving internal code structure to make easier to maintain (sustainability) priority: low Background task that doesn't need to be done right away. size: large Large project that will take a few weeks or more
Projects
None yet
Development

No branches or pull requests

3 participants