Rainforests calibration: separate models for each lead time and fixed thresholds #1940
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
benowen-bom
previously approved these changes
Oct 5, 2023
There was a problem hiding this comment.
Thanks @btrotta-bom. I'm now satisfied that all issues have been addressed and so I'm happy to approve. Overall I think these changes make significant improvements to both the method and the code presented herein.
I note that there is still an outstanding question around whether the interpolation functionality that is no longer used should be retained; MO input on this question would much appreciated.
Comment on lines
218
to
220
| warnings.warn( | ||
| "Model config does not contain lead time keys; calibration will not work" | ||
| ) |
There was a problem hiding this comment.
Thanks for adding this. I think you could go one step further and say that calibration process will fail.
There was a problem hiding this comment.
If you change this, you'll need to update the associated test.
benowen-bom
added
MO review required
gavinevans
requested changes
Oct 12, 2023
There was a problem hiding this comment.
Thanks @btrotta-bom 👍
I haven't fully wrapped my head around all these changes but these are my comments so far.
gavinevans
requested changes
Oct 13, 2023
There was a problem hiding this comment.
Thanks @btrotta-bom for the updates 👍
I've added some further minor comments.
improver_tests/calibration/rainforests_calibration/test_ApplyRainForestsCalibrationLightGBM.py
Show resolved
Hide resolved
improver_tests/calibration/rainforests_calibration/test_ApplyRainForestsCalibrationLightGBM.py
Outdated
Show resolved
Hide resolved
improver_tests/calibration/rainforests_calibration/test_ApplyRainForestsCalibrationLightGBM.py
Outdated
Show resolved
Hide resolved
improver_tests/calibration/rainforests_calibration/test_ApplyRainForestsCalibrationTreelite.py
Outdated
Show resolved
Hide resolved
gavinevans
approved these changes
Oct 16, 2023
btrotta-bom
added a commit
that referenced
this pull request
Oct 24, 2023
* clip predictions * Update rainforests docs * fix typo * Incorporate review comments * reverse change * remove unrelated change * remove unrelated change * minor updates * minor updates * fix numbered list * update --------- Co-authored-by: Belinda Trotta <[email protected]>
MoseleyS
pushed a commit
to MoseleyS/improver
that referenced
this pull request
Aug 22, 2024
… thresholds (metoppv#1940) * Updates for log transform * Changes for log transform * Changes to blend probabilties * add bounds * different bounds methodology * update monotone code * minor fixes * minor updates * Remove temporary changes * Tidy up code * tidy up code * Add tests for utilities and fix bugs. * Add tests and fix code * Reverse temporary change * Update acceptance tests * Formatting * formatting * Formatting * Fix bug where arrays are not contiguous * remove test code * Changes for separate lead times * update * clip forecast between 0 and 1 * clip forecast between 0 and 1 * convert output thresholds to array * Update tests * formatting * Use variable lower bound instead of 0 * Update checksums * merge * Remove unused code * Minor fixes * remove unused import * Handle 3H forecast * Ensure probability outputs are monotone * match lead time to nearest available model * Sort the model lead times * Remove unused tests * Fix tests * Fix tests * Formatting * Fix docstrings * Update docs * Remove unused import * Fix docstring * Add test for specifying threshold units * Fix bug and add tests * Add test * Formatting * Remove testing code * Remove testing code * Remove precipitation error bound * simplify code * Simplify code * Check same number of thresholds * Simplify creating output cube * Formatting * Use float for lead times * Update docstrings * formatting * typo * rename variables * update docstring * Restore old documentation * formatting * Fix failing acceptance test * Fix variable names * typo * formatting * Add warning if no lead time keys present in config * update checksums * fix typo * Refactor duplicated code * Fix error message * Minor updates * Change lead time to float * Fix error message * Factor out parsing of model_config_dict * Check error message * Specify format of config dict everywhere * Update comment --------- Co-authored-by: Belinda Trotta <[email protected]>
MoseleyS
pushed a commit
to MoseleyS/improver
that referenced
this pull request
Aug 22, 2024
* clip predictions * Update rainforests docs * fix typo * Incorporate review comments * reverse change * remove unrelated change * remove unrelated change * minor updates * minor updates * fix numbered list * update --------- Co-authored-by: Belinda Trotta <[email protected]>
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 needs to be reviewed by BoM first. I will add the "MO review required" tag after that has happened.
This PR makes 2 changes to the rainforests calibration code:
Testing: