Skip to content

Conversation

@pgeier
Copy link
Contributor

@pgeier pgeier commented Oct 21, 2025

Description

Removed special handling for levtype al and spherical harmonics in the entry rule mars2grib/Rules.cc.

Before AL and SH had there own path. SH fields had there own specific levtype + param -> typeOfLevel mapping.
As a result params had to be added frequently instead of making the mapping othogonal.
The split in the mapping dates back to when the rules where generated through a python script and where about to replace the existing rule files.

Contributor Declaration

By opening this pull request, I affirm the following:

  • All authors agree to the Contributor License Agreement.
  • The code follows the project's coding standards.
  • I have performed self-review and added comments where needed.
  • I have added or updated tests to verify that my changes are effective and functional.
  • I have run all existing tests and confirmed they pass.

🌈🌦️📖🚧 Documentation 🚧📖🌦️🌈
https://sites.ecmwf.int/docs/dev-section/multio/pull-requests/PR-186

@pgeier pgeier requested review from dsarmany and tweska October 21, 2025 11:54
@pgeier pgeier force-pushed the feature/simplify-mars2grib-mapping branch from a7b5861 to bfa492c Compare October 21, 2025 11:55
@codecov-commenter
Copy link

codecov-commenter commented Oct 21, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 56.61%. Comparing base (4166faa) to head (1f43047).
⚠️ Report is 1 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #186      +/-   ##
===========================================
+ Coverage    56.60%   56.61%   +0.01%     
===========================================
  Files          316      316              
  Lines        20489    20304     -185     
  Branches      1569     1569              
===========================================
- Hits         11597    11496     -101     
+ Misses        8892     8808      -84     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@MircoValentiniECMWF
Copy link
Collaborator

MircoValentiniECMWF commented Oct 21, 2025

Have you tested this change, and is it actually testable?
Please compare the generated rules against the previous JSON rules to verify consistency.
The impact of this modification appears high-risk, and the rationale behind it is not clearly explained.

@pgeier
Copy link
Contributor Author

pgeier commented Oct 21, 2025

Have you tested this change, and is it actually testable? Please compare the generated rules against the previous JSON rules to verify consistency. The impact of this modification appears high-risk, and the rationale behind it is not clearly explained.

I compared the grib1-to-grib2 outputs from /ec/project/mdst/MDST/grib1-to-grib with grib-compare and the ERA6-LWDA dump with fdb-compare.
Both are bit identical.
The JSONs with the encoder-configurations have been invalidated with the last "hotfix" one month ago.
By that time the comparison above was already used. With the further incoming changes from the migration the JSONs can't be maintained.

@pgeier pgeier force-pushed the feature/simplify-mars2grib-mapping branch from bfa492c to b17e1ca Compare October 27, 2025 10:44
Removed special handling for levtype al and spherical harmonics
@dsarmany dsarmany force-pushed the feature/simplify-mars2grib-mapping branch from b17e1ca to 1f43047 Compare October 27, 2025 12:36
@dsarmany dsarmany merged commit 4999369 into develop Oct 27, 2025
136 checks passed
@dsarmany dsarmany deleted the feature/simplify-mars2grib-mapping branch October 27, 2025 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants