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

Mass balance model with "memory", and issues for apparent_mb_from_any_mb (and others) #1755

Open
dngoldberg opened this issue Jan 1, 2025 · 1 comment · May be fixed by #1757
Open

Mass balance model with "memory", and issues for apparent_mb_from_any_mb (and others) #1755

dngoldberg opened this issue Jan 1, 2025 · 1 comment · May be fixed by #1757

Comments

@dngoldberg
Copy link
Contributor

I am helping to develop the coupling of oggm with an energy balance/snow model, and this has been implemented by creating a new class that inherits from the MassBalanceModel class.

The snow model has memory, such that in consecutive years the snow model and ice temperature state is preserved. An issue arises when get_specific_mb is meant to be called multiple times with a specific instance. As far as i can tell, this is resulting in apparent_mb_from_any_mb throwing an error in _check_terminus_mass_flux, where the exact residual has been found.. but then is no longer the correct residual since a subsequent call at this line. As I have not explored enough functionality, Im not sure but similar issues may arise elsewhere.

My question is -- what would be the best way to handle this. I could envision:

  1. adding a kwarg, e.g. reset_state=false to massbalancemodel.get_specific_mb (and get_annual_mb) to "reset" the state, and using the kwarg where appropriate (there will likely be multiple locations). It would do nothing for all exisiting MB classes.
  2. re-instantiating the mass balance model where a state reset is needed.

(1) seems better, though will look a little odd until (hopefully) our energy mb model is merged. if you are OK with this, i will create a branch and add the kwarg in all the places i think are appropriate, though this will likely not be exhaustive without further testing.

@fmaussion
Copy link
Member

Thanks for raising this issue. I don't see a problem in adding functionality for downstream projects, but a few things come to mind first:

  • Having a memory for a MB Model should be no issue. I'm not sure to understand why a subsequent call to the same year should lead to different results, since ideally your MB model will have stored (cached) the result of previous computations a a new call to a given year should simply return the same value.
  • It's a bit lazy of the apparent_mb_from_any_mb task to call the MB model twice here. Actually, we could change the code to call it only once (this call is needed, but could be done first and then the specific MB could be computed from it.

Regardless of the above, ideally a call to get_mb for a given year and geometry should always give the same result. I know that models with memory raise challenges that are not well handled by OGGM at the moment, and eventually some form of "reset" logic might be needed, but the reason here seems not yet justified to me? To discuss.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants