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

Jump to bottom

Recipes that include variables with generic level coordinates (e.g. cl) might not work anymore #1903

Closed
schlunma opened this issue Nov 16, 2020 · 6 comments · Fixed by ESMValGroup/ESMValCore#882
Closed

Recipes that include variables with generic level coordinates (e.g. cl) might not work anymore #1903

schlunma opened this issue Nov 16, 2020 · 6 comments · Fixed by ESMValGroup/ESMValCore#882
Labels
bug
Projects
November 2020

Comments

@schlunma
Copy link
Contributor

After the merge of ESMValGroup/ESMValCore#598 recipes which use variables with generic level coordinates (e.g. cl) might not work anymore, since that PR included a checker for issues that have been detected before.

Example recipe:
https://github.com/ESMValGroup/ESMValTool/blob/master/esmvaltool/recipes/recipe_ecs_constraints.yml.

Example issues:

esmvalcore.cmor.check.CMORCheckError: There were errors in variable cl:
lev: units should be 1, not hPa
 lev: has values < valid_min = 0.0
@schlunma schlunma added the bug label Nov 16, 2020
@schlunma schlunma mentioned this issue Nov 16, 2020
Merged
6 tasks
@valeriupredoi
Copy link
Contributor

wait - why should lev be in units 1 and not hPa?

@valeriupredoi
Copy link
Contributor

also note that these recipes are still functional with --check_level ignore as an interim solution 👍

@schlunma
Copy link
Contributor Author

wait - why should lev be in units 1 and not hPa?

cl uses a hybrid pressure coordinate (see https://scitools.org.uk/iris/docs/latest/iris/iris/aux_factory.html#iris.aux_factory.HybridPressureFactory), so this lev coordinate is only an auxiliary coordinate used to define the actual air_pressure coordinate.

@valeriupredoi
Copy link
Contributor

ach so, fair dos, the intricacies of CMOR are getting at me, cheers dude 🍺

@schlunma schlunma added this to In Progress in November 2020 Nov 25, 2020
This was referenced Nov 25, 2020
Merged
Merged
@nielsdrost nielsdrost mentioned this issue Dec 2, 2020
Merged
19 tasks
@schlunma schlunma mentioned this issue Jan 20, 2021
Merged
9 tasks
@schlunma schlunma mentioned this issue Feb 5, 2021
Merged
14 tasks
@bouweandela
Copy link
Member

@schlunma Is this still an issue?

@schlunma
Copy link
Contributor Author

Yes!

November 2020 automation moved this from In Progress to Merged Mar 11, 2021
valeriupredoi added a commit that referenced this issue Nov 2, 2022
@schlunma @valeriupredoi
Make results of recipes schlund20jgr_*.yml deterministic (#2900)
97878de 
<!--
    Thank you for contributing to our project!

Please do not delete this text completely, but read the text below and
keep
items that seem relevant. If in doubt, just keep everything and add your
    own text at the top, a reviewer will update the checklist for you.

While the checklist is intended to be filled in by the technical and
scientific
reviewers, it is the responsibility of the author of the pull request to
make
    sure all items on it are properly implemented.
-->

## Description

### Major changes

- Introduction of the option `random_state` to the MLR diagnostics which
can be used to make the results of the recipe deterministic, i.e., the
numerical results are identical across runs now.
- Coordinate ordering in `mlr/postprocess.py` is now fixed to make the
output of this diagnostic reproducible (see
#2901).

--> As a result, **all** plots and most netCDF files can now be compared
correctly with the recipe comparison tool. **Note that some netCDF files
are still not comparable (mainly due to differing attributes, see
#2901

### Minor changes

In addition, this PR removes the usage of deprecated features and solves
a bunch of Codacy issues:

- Removal of deprecated
`sklearn.utils.metaestimators.if_delegate_has_method`
- Removal of deprecated `sklearn.inspection.plot_partial_dependence`
- Removal of deprecated `pandas.DataFrame.append`
- Removal of deprecated `verbose` argument for `SimpleImputer`
- Fixed code that lead to warning about `X does not have valid feature
names, but SimpleImputer was fitted with feature names`
- Removal of `GradientBoostingRegressor.loss_`
- Fixed Codacy issue about using `raise Exception from ...`
- Fixed Codacy issue about using f-strings

<!--
Please describe your changes here, especially focusing on why this pull
request makes
    ESMValTool better and what problem it solves.

Before you start, please read our contribution guidelines:
https://docs.esmvaltool.org/en/latest/community/

Please fill in the GitHub issue that is closed by this pull request,
e.g. Closes #1903
-->

***

- Closes #2889
- Link to documentation:
https://esmvaltool--2900.org.readthedocs.build/en/2900/api/esmvaltool.diag_scripts.mlr/models.html#optional-parameters-for-class-initialization

* * *

## Before you get started

<!--
Please discuss your idea with the development team before getting
started,
to avoid disappointment or unnecessary work later. The way to do this is
    to open a new issue on GitHub.
-->

- [x] [☝ Create an
issue](https://docs.esmvaltool.org/en/latest/community/code_documentation.html#contributing-code-and-documentation)
to discuss what you are going to do

## Checklist

It is the responsibility of the author to make sure the pull request is
ready to review. The icons indicate whether the item will be subject to
the [🛠 Technical][1] or [🧪 Scientific][2] review.

<!-- The next two lines turn the 🛠 and 🧪 below into hyperlinks -->
[1]:
https://docs.esmvaltool.org/en/latest/community/review.html#technical-review
[2]:
https://docs.esmvaltool.org/en/latest/community/review.html#scientific-review

- [x] [🛠][1] This pull request has a [descriptive
title](https://docs.esmvaltool.org/en/latest/community/code_documentation.html#pull-request-title)
- [x] [🛠][1] Code is written according to the [code quality
guidelines](https://docs.esmvaltool.org/en/latest/community/code_documentation.html#code-quality)
- [x] [🛠][1]
[Documentation](https://docs.esmvaltool.org/en/latest/community/code_documentation.html#documentation)
is available
- [x] [🛠][1]
[Tests](https://docs.esmvaltool.org/en/latest/community/code_documentation.html#tests)
run successfully
- [x] [🛠][1] The [list of
authors](https://docs.esmvaltool.org/en/latest/community/code_documentation.html#list-of-authors)
is up to date
- [x] [🛠][1] Any changed dependencies have been [added or removed
correctly](https://docs.esmvaltool.org/en/latest/community/code_documentation.html#dependencies)
- [x] [🛠][1] All [checks below this pull
request](https://docs.esmvaltool.org/en/latest/community/code_documentation.html#pull-request-checks)
were successful

### [New or updated
recipe/diagnostic](https://docs.esmvaltool.org/en/latest/community/diagnostic.html)

- [x] [🧪][2] [Recipe runs
successfully](https://docs.esmvaltool.org/en/latest/community/diagnostic.html#testing-recipes)
- [x] [🧪][2] [Recipe is well
documented](https://docs.esmvaltool.org/en/latest/community/diagnostic.html#recipe-and-diagnostic-documentation)
- [x] [🧪][2] [Figure(s) and
data](https://docs.esmvaltool.org/en/latest/community/diagnostic.html#diagnostic-output)
look as expected from literature
- [x] [🛠][1] [Provenance
information](https://docs.esmvaltool.org/en/latest/community/diagnostic.html#recording-provenance)
has been added


***

To help with the number of pull requests:

- 🙏 We kindly ask you to
[review](https://docs.esmvaltool.org/en/latest/community/review.html#review-of-pull-requests)
two other [open pull
requests](https://github.com/ESMValGroup/ESMValTool/pulls) in this
repository

<!--
If you need help with any of the items on the checklists above, please
do not hesitate to ask by commenting in the issue or pull request.
-->

Co-authored-by: Valeriu Predoi <valeriu.predoi@gmail.com>
schlunma pushed a commit that referenced this issue Nov 7, 2022
@valeriupredoi @bouweandela
Remove pytest-flake8 plugin and call flake8 directly (#2904)
ccab48b 
<!--
    Thank you for contributing to our project!

Please do not delete this text completely, but read the text below and
keep
items that seem relevant. If in doubt, just keep everything and add your
    own text at the top, a reviewer will update the checklist for you.

While the checklist is intended to be filled in by the technical and
scientific
reviewers, it is the responsibility of the author of the pull request to
make
    sure all items on it are properly implemented.
-->

## Description

<!--
Please describe your changes here, especially focusing on why this pull
request makes
    ESMValTool better and what problem it solves.

Before you start, please read our contribution guidelines:
https://docs.esmvaltool.org/en/latest/community/

Please fill in the GitHub issue that is closed by this pull request,
e.g. Closes #1903
-->

Sister PR to ESMValGroup/ESMValCore#1722 - quite
needed since the conda-forge package needed some involved pinning of the
flake8 and pytest-flake8 packages. Discussed this and @bouweandela
provided a working solution to the problem of the `pytest_flake8` plugin
being more and more at odds with the mother package `flake8`. Currently
the main `flake8` package (and only one as of this PR) is pinned to <5,
there are still some issues with upper versions but we should have in
mind to unpin and test in the future.

- Closes #2897 
- Link to documentation: No need for it, no change to actual docs

* * *

## Before you get started

<!--
Please discuss your idea with the development team before getting
started,
to avoid disappointment or unnecessary work later. The way to do this is
    to open a new issue on GitHub.
-->

- [x] [☝ Create an
issue](https://docs.esmvaltool.org/en/latest/community/code_documentation.html#contributing-code-and-documentation)
to discuss what you are going to do

## Checklist

It is the responsibility of the author to make sure the pull request is
ready to review. The icons indicate whether the item will be subject to
the [🛠 Technical][1] or [🧪 Scientific][2] review.

<!-- The next two lines turn the 🛠 and 🧪 below into hyperlinks -->
[1]:
https://docs.esmvaltool.org/en/latest/community/review.html#technical-review
[2]:
https://docs.esmvaltool.org/en/latest/community/review.html#scientific-review

- [x] [🛠][1] This pull request has a [descriptive
title](https://docs.esmvaltool.org/en/latest/community/code_documentation.html#pull-request-title)
- [x] [🛠][1] The [list of
authors](https://docs.esmvaltool.org/en/latest/community/code_documentation.html#list-of-authors)
is up to date
- [x] [🛠][1] Any changed dependencies have been [added or removed
correctly](https://docs.esmvaltool.org/en/latest/community/code_documentation.html#dependencies)
- [x] [🛠][1] All [checks below this pull
request](https://docs.esmvaltool.org/en/latest/community/code_documentation.html#pull-request-checks)
were successful

Co-authored-by: Bouwe Andela <b.andela@esciencecenter.nl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
No open projects
November 2020
Merged
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Added fixes for hybrid level coordinates of CESM2 models ESMValGroup/ESMValCore
3 participants
@bouweandela @valeriupredoi @schlunma