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

Fixes for sos and siconc of BCC models #1090

Merged
merged 7 commits into from
May 17, 2021
Merged

Fixes for sos and siconc of BCC models #1090

merged 7 commits into from
May 17, 2021

Conversation

remi-kazeroni
Copy link
Contributor

@remi-kazeroni remi-kazeroni commented Apr 28, 2021

Description

Fixes for sos and siconc of BCC-ESM1 model and sos of BCC-CSM2 model.

Closes #1089


Before you get started

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 or 🧪 Scientific review.


To help with the number pull requests:

@remi-kazeroni remi-kazeroni requested a review from jvegreg as a code owner April 28, 2021 15:18
@remi-kazeroni remi-kazeroni self-assigned this Apr 28, 2021
@remi-kazeroni remi-kazeroni added AR6 Necessary changes for IPCC AR6 fix for dataset Related to dataset-specific fix files labels Apr 28, 2021
@remi-kazeroni remi-kazeroni added this to the IPCC AR6 milestone Apr 28, 2021
@schlunma
Copy link
Contributor

Hi @remi-kazeroni, I just opened a PR that fixes the fix that is used here (#1092). Once this is merged the fixes you introduce here are as simple as

Sos = OceanFixGrid
Siconc = OceanFixGrid

Would it be okay for you to wait with this PR until #1092 is merged? I will review it as soon as possible.

If this is really urgent I can have a look at it right now and adapt #1092 accordingly, but I think the other way round is easier 😄

@schlunma schlunma self-requested a review April 29, 2021 10:41
@remi-kazeroni
Copy link
Contributor Author

Hi @remi-kazeroni, I just opened a PR that fixes the fix that is used here (#1092). Once this is merged the fixes you introduce here are as simple as

Sos = OceanFixGrid
Siconc = OceanFixGrid

Would it be okay for you to wait with this PR until #1092 is merged? I will review it as soon as possible.

If this is really urgent I can have a look at it right now and adapt #1092 accordingly, but I think the other way round is easier 😄

Hi @schlunma, thanks for offering to review this PR. I agree it makes totally sense to wait until #1092 is merged and then adapt this PR by using the OceanFixGrid for the variables needed here. This PR is not super urgent but the sooner, the better 👍

@schlunma
Copy link
Contributor

schlunma commented May 5, 2021

Hi @remi-kazeroni, #1092 has been merged, so you can proceed with this PR 👍

As I said, I think the fixes can be covered with

Sos = OceanFixGrid

and the tests similar to this:

def test_get_tos_fix():
"""Test getting of fix."""
fix = Fix.get_fixes('CMIP6', 'BCC-ESM1', 'Omon', 'tos')
assert fix == [Tos(None)]
def test_tos_fix():
"""Test fix for ``tos``."""
assert Tos is OceanFixGrid

@remi-kazeroni
Copy link
Contributor Author

remi-kazeroni commented May 11, 2021

@schlunma this is now ready for review. The OceanFixGrid helped a lot, in particular to improve the readability 👍

Copy link
Contributor

@valeriupredoi valeriupredoi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice work @remi-kazeroni and @schlunma 🍺

@valeriupredoi
Copy link
Contributor

@schlunma pls merge when you good, bud, I like the new fix classes 👍

@schlunma schlunma merged commit 347d19d into master May 17, 2021
@schlunma schlunma deleted the fix_bcc_sos_siconc branch May 17, 2021 15:33
senesis pushed a commit that referenced this pull request Jun 7, 2021
* sos and siconc fixed

* tests added

* test fixed

* fix flake8

* fix flake8

* fix codacy issue
zklaus pushed a commit that referenced this pull request Jun 11, 2021
* Add basic support for variable mappings

* Add first era5 mapping

* Find files for CMIP6 DCPP startdates (#771)

* First attempte

* Do not require start and end years, add them later

* Correct condition

* Avoid key error in fx variables

* Consider two possible paths

* Fix function name

* Fix variable name

* Avoid duplicates in filename

* Add test for startdate expansion

* Add test for the replace tags method

* Rename tag

* Add documentation

* Allow to load subexps per timerange or as a whole

* Fix condition

* Remove 'all_years' functionality

* Fix conditions

* Fix flake

* Remove whitespace

Co-authored-by: Javier Vegas-Regidor <javier.vegas@bsc.es>

* Skip regridding if the target grid is almost identical to the source grid (#507)

Co-authored-by: Bouwe Andela <b.andela@esciencecenter.nl>
Co-authored-by: Stef Smeets <s.smeets@esciencecenter.nl>

* Fixes for sos and siconc of BCC models (#1090)

* sos and siconc fixed

* tests added

* test fixed

* fix flake8

* fix flake8

* fix codacy issue

* Pin cf-units and fix tests (cf-units>=2.1.5) (#1140)

* pin cf-units

* pin cf-units

* fix test

* fix test

* Handle IPSL-CM6 (the feature won't actually work without #1124)

* class Huss inherits from cass Tas. Also : Fix codacy diags.

* Replace os.system() by subprocess.run()

* Fix flake8 diags

* var_mapping -> extra_facets

* Rename _config/variable_details to _config/extra_facets

* Fix doc re. lack of 'output_file as a dict', and choice of native6

* Fix codacy diags in ipsl_cm6.py

* Use project IPSLCM to handle IPSL-CM6

* Implement changes according to Bouwe's review, 2021/06/07 (except unit tests)

* Add unit tests for _fixes/ipslcm/ipsl_cm6.py

* delete esmvalcore/cmor/_fixes/native6/ipsl_cm6.py

* Delete old file esmvalcore/_config/extra_facets/native6-ipsl-cm6-mappings.yml

* Restore main versions for _recipe.py and cmor_fixes/fix.py

* Restore main version for _recipe.py

* Delete extraneous era5-mappings.yml

* Avoid using mapping_key when calling fix.get_cube_from_list()

* Empty change in fix.py for forcing codacy to re-scan it

* Polish doc

* Polish doc again

* Again...

* and again ...

* Fix typo in comment

* Fixes according to @zklaus review

* Reduce formatting changes

* Update doc/develop/fixing_data.rst

Co-authored-by: Klaus Zimmermann <klaus.zimmermann@smhi.se>

* Update doc/develop/fixing_data.rst

Co-authored-by: Klaus Zimmermann <klaus.zimmermann@smhi.se>

* Update doc/develop/fixing_data.rst

Co-authored-by: Klaus Zimmermann <klaus.zimmermann@smhi.se>

* Update doc/develop/fixing_data.rst

Co-authored-by: Klaus Zimmermann <klaus.zimmermann@smhi.se>

* Update doc/develop/fixing_data.rst

Co-authored-by: Klaus Zimmermann <klaus.zimmermann@smhi.se>

* Update doc/develop/fixing_data.rst

Co-authored-by: Klaus Zimmermann <klaus.zimmermann@smhi.se>

* Update doc/develop/fixing_data.rst

Co-authored-by: Klaus Zimmermann <klaus.zimmermann@smhi.se>

* Update doc/quickstart/find_data.rst

Co-authored-by: Klaus Zimmermann <klaus.zimmermann@smhi.se>

* Update doc/quickstart/find_data.rst

Co-authored-by: Klaus Zimmermann <klaus.zimmermann@smhi.se>

* Minor formatting improvements

* Organize mapping file in each realm in two sections (CMIP6 and IPSL)

Co-authored-by: Klaus Zimmermann <klaus.zimmermann@smhi.se>
Co-authored-by: sloosvel <45196700+sloosvel@users.noreply.github.com>
Co-authored-by: Javier Vegas-Regidor <javier.vegas@bsc.es>
Co-authored-by: Benjamin Müller <b.mueller@iggf.geo.uni-muenchen.de>
Co-authored-by: Bouwe Andela <b.andela@esciencecenter.nl>
Co-authored-by: Stef Smeets <s.smeets@esciencecenter.nl>
Co-authored-by: Rémi Kazeroni <70641264+remi-kazeroni@users.noreply.github.com>
Co-authored-by: Valeriu Predoi <valeriu.predoi@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AR6 Necessary changes for IPCC AR6 fix for dataset Related to dataset-specific fix files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dataset problem: Issues with latitude and longitude for sos and siconc of BCCs CMIP6 models
3 participants