OGGM / oggm

Open Global Glacier Model
http://oggm.org
BSD 3-Clause "New" or "Revised" License
222 stars 106 forks source link

more flexibility for the task apparent_mb_from_any_mb() #1743

Closed bearecinos closed 1 month ago

bearecinos commented 1 month ago

@fmaussion (cc. @dngoldberg )

Hi we need to be able to decide which flowlines we pass to the apparent_mb_from_any_mb()

In FSM we need to pass the flowlines which have bin_area_m2 information so we need fls = gdir.read_pickle('model_flowlines') rather than fls = gdir.read_pickle('inversion_flowlines') as is coded ATM, the code works if I do this small modifications:

def apparent_mb_from_any_mb(gdir, mb_model=None,
                            mb_model_class=MonthlyTIModel,
                            mb_years=None,
                            fls=None):

....

# For each flowline compute the apparent MB
    if fls is None:
        fls = gdir.read_pickle('inversion_flowlines')

else the code will use the fls that we decide to pass via

fls = gdir.read_pickle('model_flowlines')
tasks.apparent_mb_from_any_mb(gdir, mb_model=mass_balance, fls=fls)

I can make PR for this... what do you think?

dngoldberg commented 1 month ago

hey @bearecinos that looks fairly similar to the issue i found! im happy with that, but fabi needs to decide.

if this approach can't be used... well, we only would need bin_area_m2 if there needs to be runoff data written to file (which richard will do within FSM, as i understand). when we do a thickness inversion this wouldnt be needed, so there may be another way around this -- though yours i think is the simplest.

interesting - i didn't know the two flowline files differed in this way...

fmaussion commented 1 month ago

Thanks @bearecinos - While I can see why you need this, passing model_flowlines goes against the normal OGGM workflow which would be to compute the inversion before generating the model_flowlines. It's not a very big deal, but better would be for the FSM class to be able to deal with inversion flowlines as well, or for inversion flowlines to have a bin_area_m2 attribute (this can be computed trivially from these as well).

The only reason I could see why this creates difficulties is that for apparent_mb_from_any_mb the computational domain is smaller (only for the glacier area) than for the model flowlines (glacier + downstream line).

Does this make sense?

bearecinos commented 1 month ago

Well the main thing is that we need to pass to FSM flowline information on surface_h and bin_area_m2 if those two variables can be found in the inversion_flowlines that would be the ideal thing to pass? How can I put these variables there? is there a specific cfg.PARAMS for this? or this means we have to modify the code that writes inversion_flowlines. I dont think the domain is an issue for the inversion? because once we have done that .. we can create model_flowlines from init_present_time_glacier() and that has (glacier + downstream line)?

Hopefully I make sense too 😅 ...

fmaussion commented 1 month ago

The inversion flowlines have everything you need to compute bin_area - it's just that I didn't need it before, or computed it externally when I needed it. See PR and let me know if this would work for you.

fmaussion commented 1 month ago

feel free to reopen if still relevant