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

time stepping of canopy%cansto. #505

Open
JhanSrbinovsky opened this issue Dec 9, 2024 · 4 comments · May be fixed by #506
Open

time stepping of canopy%cansto. #505

JhanSrbinovsky opened this issue Dec 9, 2024 · 4 comments · May be fixed by #506

Comments

@JhanSrbinovsky
Copy link
Collaborator

JhanSrbinovsky commented Dec 9, 2024

WRT to oldcansto% (re-)initialization within define canopy. NOT the more thorough treatment of %cansto in general AS DISCUSSED IN #162.

@har917 it isnt active in the AM3 model, or the CMIP6 version of ESM1.5. It is blocked by the IF condition. Although it is active in the ESM1.5+CABLE3 version. Probably because offline was doing it. I think your 1,2,3 are in place, I'll have to double check and get back yo you. For 1 however I think my intention was to move it back to offline/cbm where the time-stepping in cbm is the same as in define_canopy anyway and have the same shared canopy for everyone?

Originally posted by @JhanSrbinovsky in #500

@JhanSrbinovsky
Copy link
Collaborator Author

JhanSrbinovsky commented Dec 9, 2024

in cable_serial, within the timestep loop, canopy%oldcansto is set to %cansto (i.e. oldcansto=cansto) BEFORE cbm is called. So then when it gets to define_canopy it is just swapping the same thing back i.e. cansto=oldcansto

serial

oldcansto=cansto
cbm

define_canopy

cansto=oldcansto
cansto

In CM2:
explicit driver

oldcansto=cansto
cbm

define_canopy

cansto

implicit driver

PB_oldcansto=oldcansto(from beginning of timestep)
cansto=oldcansto
cansto
oldcansto=PBoldcansto
cansto=oldcansto
cansto

cansto is effectively updated once per timestep. we effectively save it to the restart as/thru canopy_tile (UM field)

ESM1.5+cable3 has:

explicit driver

oldcansto=cansto
cbm

define_canopy

cansto=oldcansto(from beginning of timestep)
cansto

implicit driver

cansto=oldcansto
cbm

define_canopy

cansto=oldcansto(from beginning of timestep)
cansto

So think they are all OK, albeit awkward, and the cansto=oldcansto line in define_canopy can be removed @har917 ?

@har917
Copy link
Collaborator

har917 commented Dec 9, 2024

@JhanSrbinovsky This looks okay (for now) but the complexity means that it is prone to be confused and errors creeping in during the future.

Perhaps one thing to check: at the moment we're only placing oldcansto in the prognostic bank (not both). I think we're okay - and changing things now is likely to


However if we want to clear things up a bit (in the short-term) I would be looking to :

  1. move the pre-cbm setting of oldcansto = cansto into define_canopy,
  2. removing the cansto=oldcansto in define_canopy, and
  3. adding (in explicit_driver most likely) cansto=oldcansto as needed to keep the logic.

i.e. for serial it goes:

  • cbm [ define_canopy [ oldcansto = cansto then update cansto ] ]

in ESM1.6 it goes

  • explicit_driver [ cbm [ define_canopy [ oldcansto = cansto then update cansto] ] cansto=oldcansto ]
  • implicit_driver [ cbm [ define_canopy [ oldcansto = cansto (which is still cansto from the start of time step) then update cansto] ] ]

and in CM2/AM3 it goes

  • explicit_driver [ cbm [ define_canopy [ oldcansto = cansto then update cansto] ] cansto=oldcansto ]
  • 1: implicit_driver [ PBcansto=cansto & PBocansto = oldcansto [ cbm [ define_canopy [ oldcansto = cansto then update cansto] ] ], cansto=PBcansto & oldcansto = PBocansto ]
  • 2: implicit_driver [ PBcansto=cansto & PBocansto = oldcansto [ cbm [ define_canopy [ oldcansto = cansto then update cansto] ] ] ]

In the longer-term we should be looking to rewrite the code in define_canopy that evaluates the water fluxes (throughfall, evaporation from a wet canopy etc.) and updates cansto so that the fluxes come out of define_Canopy but cansto itself is time stepped separately (in the same/similar way as the soil moisture).

@JhanSrbinovsky
Copy link
Collaborator Author

In the CM3/AM3 I think we only need to PB cansto?

@har917
Copy link
Collaborator

har917 commented Dec 9, 2024

In the CM3/AM3 I think we only need to PB cansto?

At the moment we are PB'ing oldcansto not cansto - I think this works as you've laid it out!!

It is confusing - especially the cansto = oldcansto lines that appear in some of the applications of define_canopy and not others.

More generally I find the mix of where the cansto variables are used/timestepped within cbm and define_canopy to be odd - and something that should be tackeld down the track

Actually I suggest that we work towards removing all instances of if um_explicit, if um_implicit etc. from the science routines BUT that's for later.

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 a pull request may close this issue.

2 participants