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

Bugfix in merge_gridded_data #1697

Merged
merged 4 commits into from
Apr 25, 2024

Conversation

pat-schmitt
Copy link
Member

@pat-schmitt pat-schmitt commented Mar 12, 2024

This is a bugfix for the problem reported here #1694. The problem was a division through zero when we try to preserve totals but the glacier has completely disappeared (as described in the issue). I adapted one test and could reproduce and solve the problem. Thanks @bpelto for investigating!

Further, I included here an option for smoothing when merging simulated thickness. This is useful to make the ice thicknesses less pixelated for 3D visualisations. I will add this to the tutorials.

Closes #1694

  • Tests added/passed
  • Fully documented
  • Entry in whats-new.rst

@@ -1958,6 +1963,7 @@ def reproject_gridded_data_variable_to_grid(gdir,
if preserve_totals:
# only preserve for float variables
if not np.issubdtype(data, np.integer):
# do we want to do smoothing?

Choose a reason for hiding this comment

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

Suggested change
# do we want to do smoothing?
# TODO: do we want to do smoothing?

because is an open question and not a comment ;-) (also it is also a comment)

@fmaussion
Copy link
Member

Thanks @pat-schmitt ! From the doctext I'm not sure why smoothing can only be done when preserving totals is true? Otherwise looks good to me.

@pat-schmitt
Copy link
Member Author

I made this choice because the smoothing effect is quite strong. If we don't preserve totals, the resulting smoothed volume will deviate even further from the actual modeled value. However, I could also adjust the approach so that smoothing is applied without preserving totals.

@fmaussion
Copy link
Member

I don't mind too much - best is to keep it easy, so let's stick to this for now. Merge when you are ready!

@pat-schmitt pat-schmitt merged commit 7489be1 into OGGM:master Apr 25, 2024
27 checks passed
@pat-schmitt pat-schmitt deleted the fix_bug_merge_gridded_data branch April 25, 2024 10:08
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.

merge_simulated_thickness produces all NaN for all glaciers as soon as one disappears
3 participants