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

Changes in climate and massbalance internals: part 1 #509

Merged
merged 10 commits into from Aug 8, 2018

Conversation

Projects
None yet
2 participants
@fmaussion
Copy link
Member

commented Jul 31, 2018

  • Closes #483
  • Tests added/passed
  • Fully documented, including whats-new.rst for all changes

This PR is a first step towards some major changes in the mass-balance internals, not without consequences on the results of the model (unfortunately)

Some of the most important changes:

  • temperature gradients are now added to the climate files only if computed from local regression (never used operationally). This was done in order to treat the temperature gradient parameter like any other parameter for the the model cross-validation and calibration
  • precipitation factors are now also treated as a normal MB model parameter. They had a special treatment in the previous implementation because I thought it would be possible to decide of the factor automatically, but this never worked out. Hence, the code is now simplified and there is no more difference between, say, precip factor and temperature gradient or melt temperature
  • when more than one tstar is available (the case for many glaciers), we now take the one with the smallest absolute bias (what Ben does) instead of the fancy global search I was doing before. This makes the results MUCH more predictable (and reproducible), and makes the cross-validation faster. This is what will have strong consequences on the model output, and needs to be evaluated
  • the task distribute_tstar is now depcrecated in favor of the entity task local_mustar, which does
    exactly the same job but in parallel

Altogether this goes in the direction of the Zen: more explicit, simpler code.

fmaussion added some commits Jul 31, 2018

@@ -24,6 +24,9 @@ install:
- "SET PATH=%PYTHON%\\Scripts;%PYTHON%;%PATH%"
- "SET GDAL_DATA=%PYTHON%\\Library\\share\\gdal"

# Tell appveyor to run the slow tests
- "SET OGGM_SLOW_TESTS=true"

This comment has been minimized.

Copy link
@TimoRoth

TimoRoth Aug 7, 2018

Contributor

You can safely drop this now

fmaussion added some commits Aug 7, 2018

@fmaussion

This comment has been minimized.

Copy link
Member Author

commented Aug 7, 2018

@TimoRoth are you sure that the --mpl option is used on Travis currently?

@TimoRoth

This comment has been minimized.

Copy link
Contributor

commented Aug 7, 2018

Seems like moving the script to an actual script broke that, since Docker didn't forward it so far.
Now it does, so it should work again.

@fmaussion

This comment has been minimized.

Copy link
Member Author

commented Aug 8, 2018

@matthiasdusch this is ready for review now - can I ask you to go through the changes (I know its a lot) and simply ask when something is unclear or seems wrong? This will help me a lot and might help future readers interested in the changes.

@fmaussion fmaussion merged commit 7c8b233 into OGGM:master Aug 8, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@fmaussion

This comment has been minimized.

Copy link
Member Author

commented Aug 8, 2018

In it is!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.