Daily TI Model and W5E5 daily data #1769
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Adds the DailyTIModel from OGGM/massbalance-sandbox. Relies on subclass' methods instead of if statements in parent. No breaking changes! The output from DailyTIModel is slightly different as it switches to the Julian year for calculating melt_f (mean percentage difference -0.23%).
Some tests are skipped as daily data is not yet implemented into ref_mb_data.
Links ``process_w5e5_data`` to ``process_gswp3_w5e5_data_daily`` to preserve workflow for other daily resolution datasets. Data is currently pulled from ~dtcg until it is available on OGGM. Refactors: - Duplicated code - URL checking in conftest Removes: - Unused gradient computations, unused imports. Refs: OGGM#1750
Fixes missing ``climate_historical_daily`` file and monthly data passed to daily model.
Removes support for hydroyear conversion as data now follow Julian calendar. Adds tests for W5E5 shop. Bugs: Some statistical functions do not yet support daily data.
Adds daily specific mass balance to output via ``get_specific_mb_daily``. Daily MB is not fully integrated with the rest of OGGM, nor with leap years. Bug: ``get_annual_mb`` and hence ``get_specific_mb`` skips the last month of data.
Fixes bug where the last month of data was not downloaded from the shop, which then affected annual mass balance calculations.
Fixes bug where get_annual_mb and get_daily_mb would not return the same annual mass balance. Refactors some vectorisations for performance.
Refactors ``get_specific_mb`` functions as the upper bound of the stack is always known. Performance is ~2x faster.
No need for recursion as bounds are known. Performance is 2x faster.
Refactors method and variable names for readability. Fixes typos in docstrings which caused formatting to fail.
Begins deprecating the ``upscale_factor`` to automatically account for leap years. Refactors for legibility: methods primarily rely on inheritance. Fixes: - references pointing to DailySfcTIModel in a different branch. - leap years not accounted for in tests. Refs: OGGM#1750
Fixes gswp3-w5e5 url and associated tests pointing to w5e5 data. Refactors some docstrings and tests.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds support for data at daily resolution and a daily temperature index model (#1750).
Changes
Adds:
process_w5e5_dataorprocess_gswp3_w5e5_data_daily. This createsclimate_historical_daily.Refactors:
MassBalanceModel,MonthlyTIModel. My strategy has been to refactor their methods into smaller ones, and overload with DailyTI's methods. This may need to change when implementing surface tracking, but it'll reduce merge conflicts downstream.conftest.patch_data_urls.Points for discussion
Storing a daily variant
mb_calib.jsonwill be trickier to implement - a few flowline functions in OGGM use a hardcoded file path e.g. when merging glaciers. Currently this preventsTestInitPresentDayFlowline.test_present_time_glacier_massbalancefrom passing with daily data without overwritingmb_calib, so I've marked this test as xfail. This is partly related to #1677.Should there be a separate default
mb_calibavailable for daily data instead of expecting the user to calibrate their model? Is it worth the effort/bandwidth?shop.w5e5.GSWP3_W5E5_SERVERandconftest.patch_data_urlslink to ~dtcg on the cluster for W5E5 data. Should this instead link to an OGGM equivalent?The
upscale_factorin massbalance-sandbox can be deprecated, as DailyTI no longer relies onmonthly_melt_f. I have kept it as a fallback in case a calibration method does not account for leap years.There is a negligible discrepancy O(-15) between the annual smb calculated from
get_specific_mbandget_specific_mb_daily. I think this is just because of when means/sums are taken, which cause different floating point errors.I've added some type aliases to the autodoc configuration in
docs/conf.py. This imports the module under the hood, so it may be faster/safer to build docs without this.Refs: #844, #950, #1349, #1574, #1750
whats-new.rst