Consolidate all local-to-WMO unit mapping rules in compile-time rule system#261
Open
Consolidate all local-to-WMO unit mapping rules in compile-time rule system#261
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR migrates the remaining “local-to-WMO” unit/parameter mappings from runtime YAML into the compile-time wmoUnitMapping() rule list, and aligns ozone/albedo scalings with updated definitions.
Changes:
- Add many new WMO unit mapping rules in
wmoUnitMapping()(water-cycle, albedo, time-mean rates, time-mean cloud/albedo). - Update total column ozone (param 206) scaling to 46698.05 and add climatological albedo (param 174) mapping.
- Extend mapping tests and update
local-to-wmo.yamlaccordingly.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
src/multio/mars2mars/Rules.cc |
Adds/organizes new compile-time mapping rules and updates ozone scaling (but currently introduces build-breaking type/header changes). |
tests/multio/mars2mars/test_multio_mars2mars_mappings.cc |
Adds tests for new mappings (but ozone expectation is wrong and param 174 coverage is missing). |
share/multio/scalings/local-to-wmo.yaml |
Updates ozone scaling and enables param 174 mapping entry. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #261 +/- ##
===========================================
+ Coverage 56.27% 60.49% +4.21%
===========================================
Files 323 327 +4
Lines 21132 22337 +1205
Branches 1686 1720 +34
===========================================
+ Hits 11893 13512 +1619
+ Misses 9239 8825 -414 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Prefix all misc keys and add `dumpUnscopedRecord` method
Access new keys through records to apply checks and defaults
Rename missingValue -> misc-missingValue
Also default bitmapPresent to false instead of allowing it to be unset
Implement the 36 missing param mappings from share/multio/scalings/local-to-wmo.yaml as compile-time rules in wmoUnitMapping(). Add corresponding tests for all new mappings.
Add rule for climatological albedo (174 -> 260509, x100), fix total column ozone scaling to 46698.05 (kg/m2 -> DU) removing duplicate definition, and update stale comment for param 174.
Update scaling for param 206 (tco3 -> tcioz) from 1000 to 46698.05 (kg/m2 -> Dobson Units) to match the compile-time rule.
172189 (msdr) is a GRIB1-only parameter with no straightforward unit scaling to its GRIB2 counterpart 235340 (avg_cossza).
Replace all dm::MarsRecord references with dm::FullMarsRecord to match
the renamed type in the datamod layer, and update the greaterThan step
comparison to use dm::TimeDuration{std::chrono::hours{1}} instead of
std::int64_t{1} to match the new Entry<TimeDuration> step field type.
… SFC test list - Remove mars2mars/encode-mtg2 from BUILD_MULTIOM block in tests/CMakeLists.txt (already reachable through nested add_subdirectory(multio)) - Add encode-mtg2 subdirectory to tests/multio/action/CMakeLists.txt - Exclude param 141 (snow depth) from SFC test params to avoid mars2mars cross-rule-list conflict (141→228141 WMO remap hits SOL fixup rule)
dsarmany
force-pushed
the
bugfix/consolidate-wmo-units
branch
from
d28e291 to
a87bb86
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
dsarmany
force-pushed
the
bugfix/consolidate-wmo-units
branch
from
17d01a6 to
09b4dcf
Compare
This file contains hidden or 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
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.
Summary
share/multio/scalings/local-to-wmo.yamlas compile-time rules inwmoUnitMapping(), covering cloud cover, precipitation/water cycle, albedo (ERA), ERA water-cycle params, time-mean rates, and time-mean cloud/albedo fieldstest_multio_mars2mars_mappings.ccFiles changed
src/multio/mars2mars/Rules.cc-- 36 new rule functions + registration inwmoUnitMapping()tests/multio/mars2mars/test_multio_mars2mars_mappings.cc-- tests for all new rulesshare/multio/scalings/local-to-wmo.yaml-- fix ozone scaling to 46698.05🌈🌦️📖🚧 Documentation 🚧📖🌦️🌈
https://sites.ecmwf.int/docs/dev-section/multio/pull-requests/PR-261