Skip to content

Conversation

@pat-schmitt
Copy link
Member

@pat-schmitt pat-schmitt commented May 12, 2025

This is a first package of implementing the new way of parameter handling in OGGM. It is an adapted version of parts from PR #1756.

Currently those new settings handling is only included in massbalance.py and flowline.py (including their dependencies from other parts of the model). Further work is needed in the future to make everything consistent again.

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

@pat-schmitt
Copy link
Member Author

Ok, this is ready for review.

I included now some parts which are not ideal. The main troublemaker is the implementation of the dynamic spinup (which I plan to refactor a lot during summer). Consequences are that I needed to include a read from the settings yml file, whenever a parameter is obtained. This ensures that the provided parameter is always in sync with the file, even if their are two objects open, which are relying on the same yml file.

Further, for ensuring backwards compatibility, I used cfg.PARAMS.copy() at some points. I used the copy() only because otherwise the log will be flooded with messages when a object is copied (you see this if you do copy.deepcopy(cfg.PARAMS)). This is again something which is used in the dynamic spinup (really needs some refactoring).

However, so far the mass balance and the dynamics are working with the new settings file and I think this are the most critical parts for implementing the new daily massbalance model.

pat-schmitt and others added 2 commits June 24, 2025 17:17
* add optional custom grid for merging gridded data

* fix bug when reprojecting relatively small grid to a larger one

* fix bug when merging 3D data

* added whats-new
@pat-schmitt pat-schmitt merged commit dc2c625 into OGGM:dev Jul 9, 2025
27 checks passed
@pat-schmitt pat-schmitt deleted the new_settings_handling branch July 9, 2025 09:36
pat-schmitt added a commit to pat-schmitt/oggm that referenced this pull request Jul 22, 2025
* add new settings to massalance and flowline

* add automatic testing for dev branch

* fix tests

* more test fixing

* more test fixing

* Compile fl diagnostics (OGGM#1778)

* Move copdownload to OpenTopography (OGGM#1773)

* Allowe to use a mini params file during initialization (OGGM#1776)

* Override default DEM source in prepro dirs (OGGM#1781)

* add optional custom grid for merging gridded data (OGGM#1779)

* add optional custom grid for merging gridded data

* fix bug when reprojecting relatively small grid to a larger one

* fix bug when merging 3D data

* added whats-new

---------

Co-authored-by: Fabien Maussion <fabien.maussion@bristol.ac.uk>
pat-schmitt added a commit that referenced this pull request Jul 22, 2025
* add new settings to massalance and flowline

* add automatic testing for dev branch

* fix tests

* more test fixing

* more test fixing

* Compile fl diagnostics (#1778)

* Move copdownload to OpenTopography (#1773)

* Allowe to use a mini params file during initialization (#1776)

* Override default DEM source in prepro dirs (#1781)

* add optional custom grid for merging gridded data (#1779)

* add optional custom grid for merging gridded data

* fix bug when reprojecting relatively small grid to a larger one

* fix bug when merging 3D data

* added whats-new

---------

Co-authored-by: Fabien Maussion <fabien.maussion@bristol.ac.uk>
pat-schmitt added a commit that referenced this pull request Sep 9, 2025
* feat: child DailyTIModel, refactor MonthlyTIModel

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%).

* tests(models): add template for DailyTIModel

Some tests are skipped as daily data is not yet implemented into
ref_mb_data.

* feat(shop): download W5E5 data at daily resolution

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: #1750

* feat: convert between hydro/julian dates

Adds support for converting between hydro/julian date arrays and vice
versa, used for daily W5E5 data.

Refs: #844, #1349

* refactor(models): MB models switch more cleanly between resolutions

Fixes missing ``climate_historical_daily`` file and monthly data passed
to daily model.

* refactor(shop): drop hydroyear support, tests

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.

* fix(massbalance): incorrect dimensions for daily climate data

* feat(massbalance): daily specific mass balance output

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.

* fix(massbalance): W5E5 shop now downloads 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.

* fix: specific mass balance not accounting for leap years

Fixes bug where get_annual_mb and get_daily_mb would not return the same
annual mass balance.

Refactors some vectorisations for performance.

* refactor(massbalance): get_specific_mb is no longer recursive

Refactors ``get_specific_mb`` functions as the upper bound of the stack is
always known. Performance is ~2x faster.

* fix(tests): incorrect reference

* fix(massbalance): deprecated conversion of arrays to scalar

* refactor(massbalance): get_ela is no longer recursive

No need for recursion as bounds are known. Performance is 2x faster.

* refactor(tests): check ela/smb output is within sensible bounds

* refactor(massbalance): method names, docstrings

Refactors method and variable names for readability.

Fixes typos in docstrings which caused formatting to fail.

* refactor: remove np.append in favour of faster lists

* feat(massbalance): support for DailyTIModel

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: #1750

* tests(massbalance): fix leap years and xfail

* chore(massbalance): clean up docs and PEP8

* fix(massbalance): gswp3 server, dl_logic

Fixes gswp3-w5e5 url and associated tests pointing to w5e5 data.

Refactors some docstrings and tests.

* docs: add entries to changelog

* fix: tests can fail if RGI data was not downloaded

Refs: DTC-Glaciers/dtcg#9, DTC-Glaciers/dtcg#10

* refactor!: deprecate upscale_factor attribute

* fix(mass balance): calibration can now use custom models

Users can now pass model arguments as kwargs to calibration functions,
which prevents the calibration model reverting to class defaults.

Refs: #1771

* tests: add DailyTI fixture, update docs

* feat: surface tracking

Adds Schuster's surface tracking model from massbalance-sandbox.

* add new settings handling to massalance and flowline (#1777)

* add new settings to massalance and flowline

* add automatic testing for dev branch

* fix tests

* more test fixing

* more test fixing

* Compile fl diagnostics (#1778)

* Move copdownload to OpenTopography (#1773)

* Allowe to use a mini params file during initialization (#1776)

* Override default DEM source in prepro dirs (#1781)

* add optional custom grid for merging gridded data (#1779)

* add optional custom grid for merging gridded data

* fix bug when reprojecting relatively small grid to a larger one

* fix bug when merging 3D data

* added whats-new

---------

Co-authored-by: Fabien Maussion <fabien.maussion@bristol.ac.uk>

* docs: update mass balance docs

* fix: deprecated filesuffix arguments

* chore: lint and doc updates, explicit signatures

* reviewed adding daily w5e5 data

* reviewed DailyTIModel

* fix tests

* fix tests

* update sample data

---------

Co-authored-by: gampnico <45390064+gampnico@users.noreply.github.com>
Co-authored-by: Fabien Maussion <fabien.maussion@bristol.ac.uk>
pat-schmitt added a commit that referenced this pull request Sep 10, 2025
* feat: child DailyTIModel, refactor MonthlyTIModel

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%).

* tests(models): add template for DailyTIModel

Some tests are skipped as daily data is not yet implemented into
ref_mb_data.

* feat(shop): download W5E5 data at daily resolution

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: #1750

* feat: convert between hydro/julian dates

Adds support for converting between hydro/julian date arrays and vice
versa, used for daily W5E5 data.

Refs: #844, #1349

* refactor(models): MB models switch more cleanly between resolutions

Fixes missing ``climate_historical_daily`` file and monthly data passed
to daily model.

* refactor(shop): drop hydroyear support, tests

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.

* fix(massbalance): incorrect dimensions for daily climate data

* feat(massbalance): daily specific mass balance output

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.

* fix(massbalance): W5E5 shop now downloads 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.

* fix: specific mass balance not accounting for leap years

Fixes bug where get_annual_mb and get_daily_mb would not return the same
annual mass balance.

Refactors some vectorisations for performance.

* refactor(massbalance): get_specific_mb is no longer recursive

Refactors ``get_specific_mb`` functions as the upper bound of the stack is
always known. Performance is ~2x faster.

* fix(tests): incorrect reference

* fix(massbalance): deprecated conversion of arrays to scalar

* refactor(massbalance): get_ela is no longer recursive

No need for recursion as bounds are known. Performance is 2x faster.

* refactor(tests): check ela/smb output is within sensible bounds

* refactor(massbalance): method names, docstrings

Refactors method and variable names for readability.

Fixes typos in docstrings which caused formatting to fail.

* refactor: remove np.append in favour of faster lists

* feat(massbalance): support for DailyTIModel

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: #1750

* tests(massbalance): fix leap years and xfail

* chore(massbalance): clean up docs and PEP8

* fix(massbalance): gswp3 server, dl_logic

Fixes gswp3-w5e5 url and associated tests pointing to w5e5 data.

Refactors some docstrings and tests.

* docs: add entries to changelog

* fix: tests can fail if RGI data was not downloaded

Refs: DTC-Glaciers/dtcg#9, DTC-Glaciers/dtcg#10

* refactor!: deprecate upscale_factor attribute

* fix(mass balance): calibration can now use custom models

Users can now pass model arguments as kwargs to calibration functions,
which prevents the calibration model reverting to class defaults.

Refs: #1771

* tests: add DailyTI fixture, update docs

* feat: surface tracking

Adds Schuster's surface tracking model from massbalance-sandbox.

* add new settings handling to massalance and flowline (#1777)

* add new settings to massalance and flowline

* add automatic testing for dev branch

* fix tests

* more test fixing

* more test fixing

* Compile fl diagnostics (#1778)

* Move copdownload to OpenTopography (#1773)

* Allowe to use a mini params file during initialization (#1776)

* Override default DEM source in prepro dirs (#1781)

* add optional custom grid for merging gridded data (#1779)

* add optional custom grid for merging gridded data

* fix bug when reprojecting relatively small grid to a larger one

* fix bug when merging 3D data

* added whats-new

---------

Co-authored-by: Fabien Maussion <fabien.maussion@bristol.ac.uk>

* docs: update mass balance docs

* fix: deprecated filesuffix arguments

* chore: lint and doc updates, explicit signatures

* merge: merge updates from feat-daily-mass-balance

* tests: update tests for daily models

* fix(tests): merge conflict solution used wrong test

* reviewed adding daily w5e5 data

* reviewed DailyTIModel

* refactor: DailySfcTIModel

Overhauls and fixes port from mb-sandbox

* add SfcTypeTIModel

* fix tests

* text fix

* adapt SfcTypeTIModel for calibration and dynamics

* fix test

* avoid using pandas when saving past smb in SfcTypeTIModel

---------

Co-authored-by: gampnico <45390064+gampnico@users.noreply.github.com>
Co-authored-by: Fabien Maussion <fabien.maussion@bristol.ac.uk>
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.

2 participants