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

Fixing of CORDEX datasets in lambert conformal conical projection #1956

Open
wants to merge 47 commits into
base: main
Choose a base branch
from

Conversation

Neah-Ko
Copy link

@Neah-Ko Neah-Ko commented Mar 3, 2023

Description

Fixing of CORDEX datasets in lambert conformal conical projection.

Some datasets have wrongly defined coordinates, especially on the boundaries.

The fix behaves this way:

  1. Checks for differences between the coordinates boundaries and the CORDEX standard domain boundaries
  • We check both geographical coordinates and projections coordinates.
    • If the coordinates are matching the standard domain, we fix the metadata and make sure there are bounds
    • This step necessitates to pass from one coordinate system to another to make comparison in the same coordinate system.
      i.e. latitude/longitude are in geographical and projection_x/y_coordinate are in lambert.
    • For now the checks works with some thresholds that could be tuned.
  1. Fix faulty coordinate
  • if both are wrong: we raise an error
  • else
    • if one is wrong and one is right, we use the right one to re-create the wrong one.

Closes #1879

Link to documentation


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.


@Neah-Ko Neah-Ko requested a review from sloosvel March 3, 2023 10:07
@Neah-Ko Neah-Ko marked this pull request as draft March 3, 2023 10:10
@codecov
Copy link

codecov bot commented Mar 6, 2023

Codecov Report

Merging #1956 (3dde4cc) into main (72b4169) will increase coverage by 0.04%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1956      +/-   ##
==========================================
+ Coverage   92.79%   92.84%   +0.04%     
==========================================
  Files         236      253      +17     
  Lines       12438    12544     +106     
==========================================
+ Hits        11542    11646     +104     
- Misses        896      898       +2     
Impacted Files Coverage Δ
esmvalcore/cmor/check.py 97.56% <ø> (-0.01%) ⬇️
...ixes/cordex/cnrm_cerfacs_cnrm_cm5/cnrm_aladin52.py 100.00% <100.00%> (ø)
...ixes/cordex/cnrm_cerfacs_cnrm_cm5/cnrm_aladin53.py 100.00% <100.00%> (ø)
...ixes/cordex/cnrm_cerfacs_cnrm_cm5/cnrm_aladin63.py 100.00% <100.00%> (ø)
...ixes/cordex/cnrm_cerfacs_cnrm_cm5/ictp_regcm4_6.py 100.00% <100.00%> (ø)
esmvalcore/cmor/_fixes/cordex/cordex_fixes.py 100.00% <100.00%> (ø)
...e/cmor/_fixes/cordex/ecmwf_eraint/cnrm_aladin52.py 100.00% <100.00%> (ø)
...e/cmor/_fixes/cordex/ecmwf_eraint/cnrm_aladin53.py 100.00% <100.00%> (ø)
...e/cmor/_fixes/cordex/ecmwf_eraint/cnrm_aladin63.py 100.00% <100.00%> (ø)
...e/cmor/_fixes/cordex/ecmwf_eraint/ictp_regcm4_6.py 100.00% <100.00%> (ø)
... and 12 more

... and 1 file with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Contributor

@sloosvel sloosvel left a comment

Choose a reason for hiding this comment

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

Thanks for the work @Neah-Ko !! Looks like we are close to a proper fix for these datasets, but first could you please address the following comments?

esmvalcore/cmor/_fixes/cordex/cordex_fixes.py Outdated Show resolved Hide resolved
esmvalcore/cmor/_fixes/cordex/cordex_fixes.py Outdated Show resolved Hide resolved
esmvalcore/cmor/_fixes/cordex/cordex_fixes.py Outdated Show resolved Hide resolved
esmvalcore/cmor/_fixes/cordex/cordex_fixes.py Outdated Show resolved Hide resolved
esmvalcore/cmor/_fixes/cordex/cordex_fixes.py Outdated Show resolved Hide resolved
…more relevant position, accounted for possible non monotonicity, refactor and style
@Neah-Ko Neah-Ko changed the title Dev cordex lambert Fixing of CORDEX datasets in lambert conformal conical projection Mar 9, 2023
@Neah-Ko Neah-Ko added the fix for dataset Related to dataset-specific fix files label Mar 9, 2023
@Neah-Ko Neah-Ko self-assigned this Mar 10, 2023
@sloosvel sloosvel marked this pull request as draft March 21, 2023 17:00
@sloosvel
Copy link
Contributor

Thanks looking into all of @Neah-Ko !! I tested a bit the fixes with data and there is a bit reorganization to be done. Also with @pepcos we discussed that maybe it would be best to apply the fixes at a dataset level at the end, because for some datasets the results look a bit shifted. I will take what you have already done that is already working nicely and adapt it a bit so that it covers the datasets we have tested.

@sloosvel sloosvel marked this pull request as ready for review March 31, 2023 14:56
@sloosvel
Copy link
Contributor

sloosvel commented Mar 31, 2023

In summary:

  • This PR introduces a general check for CORDEX datasets in an LCC system to see if the geographical coordinates are within the domain given by the specifications. The domain description is given by a set of latitudes and longitudes that describe 9 grid points and can be found in the added file npr_rcm_domain_boundaries.json taken from this document.

  • The projection coordinates in some datasets are shifted and do not match the domain described by the geographical coordinates (see some plots generated by @Neah-Ko in these notebooks. This PR introduces a fix that computes the projection coordinates from the geographical coordinates doing a crs transformation, guesses the 1D bounds, and then uses the guessed bounds 1D bounds in order to compute the geographical 2D bounds by doing a crs transformation. Coordinate metadata is also fixed. This fix has been tested in datasets ALADIN52, ALADIN53, ALADIN63, ICTP-RegCM4-6 in the EUR and MED domains.

  • There is an additonal small metadata fix in dataset DMI-HIRHAM5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix for dataset Related to dataset-specific fix files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dataset problem: CORDEX datasets in a Lambert Conformal coordinate system
2 participants