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

move negative melt_on_glacier to snowfall #1584

Merged
merged 3 commits into from May 23, 2023

Conversation

lilianschuster
Copy link
Member

In hydrological projections, negative melt_on_glacier values occurred in the past. Although the absolute values were mostly relatively small, negative melt_on_glacier (and consequently negative glacier runoff) does not makes sense.

With this pull request, no negative melt_on_glacier values should exist anymore. The previously negative values are now clipped to zero, and the negative values are added as absolute values to the snowfall_on_glacier (for mass-conservation issues).

  • There is one line in a test that still fails:

    assert_allclose(odf['tot_prcp'], odf['tot_prcp'].iloc[0])
    . This is because the total precipitation changes even in a constant climate (because of adding the negative values as absolute values to the snowfall_on_glacier variable). I am not sure if this needs to stay constant. @fmaussion, what do you think?

  • I could have a quick check if this makes a difference in the run_with_hydro output (by doing projections for one basin). And test again if the melt_on_glacier is now everywhere non-negative.

This PR is part of the ToDo-List of #1582

  • Tests added/passed (except for one line)

Copy link
Member

@fmaussion fmaussion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks lily - I didn't check the maths, but its probably worth checking on one region or two if it changes the results

oggm/tests/test_models.py Show resolved Hide resolved
@@ -4308,6 +4308,9 @@ def test_hydro_out_from_no_glacier(self, hef_gdir, inversion_params, store_month
odf['liq_prcp_on_glacier'] +
odf['snowfall_off_glacier'] +
odf['snowfall_on_glacier'])
# this test fails because snowfall_on_glacier changes, as the
# formerly negative melt_on_glacier was added to snowfall_on_glacier
# is this a strict test??? (the remaining part of the test does not fail)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it's ok to remove the first time step - maybe just check that tot_prcp is constant after the first year

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I only check now that tot_prcp is constant after the first year. This works, but I don't know if it always works. If melt_on_glacier is negative after the first year, then the tot_prcp wouldn't be constant anymore. But I guess in this idealised case, due to the constant climate, the melt_on_glacier also does not get negative?

oggm/tests/test_models.py Show resolved Hide resolved
@lilianschuster
Copy link
Member Author

I checked whether the results changed between the versions.

  • as expected, melt_on_glacier is now always non-negative (checked in RGI reg 11, 06, 15)
  • the new melt_on_glacier is thus always equal or larger than the old one, and the new snowfall_on_glacier is also equal or larger than the old one
  • (same for the monthly values)

For single glaciers and years, the runoff can be visibly different. Over the sum of an entire RGI region, the differences are almost not visible (although the relative differences can be substantial due to sometimes small absolute numbers, specifically when comparing the mean monthly cycle of melt_on_glacier and snowfall_on_glacier).

(checks are in https://nbviewer.org/urls/cluster.klima.uni-bremen.de/~lschuster/runs_oggm_v16/compare_negative_melt_on_glacier_snowfall_on_glacier.ipynb )

@fmaussion fmaussion merged commit bb940fe into OGGM:master May 23, 2023
25 checks passed
@fmaussion fmaussion mentioned this pull request Aug 14, 2023
7 tasks
@lilianschuster lilianschuster deleted the dev_neg_melt_on_glac branch August 25, 2023 09:00
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 this pull request may close these issues.

None yet

2 participants