Skip to content

Conversation

@pgeier
Copy link
Contributor

@pgeier pgeier commented Oct 17, 2025

Description

TLDR: Have a look in multio/mars2grib/Encoder.cc to see the separation of ALLOCATE, PRESET and RUNTIME.

This draft is migrating the ALLOCATE part from MULTIOM (fortran) to mars2grib (C++).

At the moment the mars2grib mapping is mapping to an intermediate configuration which is then passed to MULTIOM.
MULTIOM then sets all GRIB2 keys. This happens through a separation in many section setters that are constructed from a partial configuration (which is passed from C++). Each section setters implements multiple methods that set keys in different stages "ALLOCATE" (to set the grib2 structure), "PRESET" (to preset all keys that can be hashed), and "RUNTIME" to set all keys that eventually vary from message to message.

The division in these stages is mandatory due to avoid unwanted ECCODES sideeffects. If all keys are set directly, some keys are not found, hence a clone is required to maintain a deterministic behaviour.

Instead of reimplementing the same structure, the draft aims to separate the mapping from MARS to GRIB2 from the workarounds of setting keys to eccodes - that means the separation in stages should not dominate the overall code structure.
This is achieved by created an struct of structs called Grib2Layout. The Grib2Layout contains a set of structs which are groups of keys that are always set together. Then the mars2grib/Rules directly map to GRIB2 keys instead of an intermediate encoder configuration. As a result, after the mapping all GRIB2 keys that are intended to set are already completely mapped - no further mapping logic is happening afterwards.

In a separate step the keys are then set to a grib handle - this happens through 3 hardcoded functions allocateSample, presetSample and setSampleDetails. These functions then only set specific groups of Grib2Keys, resulting in the same setting behaviour as performed in the MULTIOM Fortran code.

Benefits:

  • Grib2Layout structure can be serialized
    • enables testing full mappings without having to compare grib2 messages
    • in case of errors, the structure can be dumped and recreated in isolated test cases
  • readback tests can be performed easily on each group after setting each group and after having set all groups to the sample (this allows testing for eccodes sideeffects)

For now draft serves as show-case for the alternative structure. After discussion it can be split up in multiple PR if desired - however as the intermediate interfacing structure can/must be changed now, in some cases multiple changes are needed all at once.

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-183

@pgeier pgeier force-pushed the feature/multiom-migrate-allocate branch from 1636338 to 922755a Compare October 17, 2025 08:19
@pgeier pgeier force-pushed the feature/multiom-migrate-allocate branch from 922755a to 4ed00c8 Compare October 27, 2025 11:49
@codecov-commenter
Copy link

codecov-commenter commented Oct 27, 2025

Codecov Report

❌ Patch coverage is 28.33333% with 172 lines in your changes missing coverage. Please review.
✅ Project coverage is 56.41%. Comparing base (4166faa) to head (4ed00c8).

Files with missing lines Patch % Lines
src/multio/datamod/types/Grib2TimeDurationUnit.cc 38.61% 62 Missing ⚠️
src/multio/datamod/types/GridType.cc 0.00% 43 Missing ⚠️
...ultio/datamod/types/TypeOfStatisticalProcessing.cc 0.00% 38 Missing ⚠️
src/multio/datamod/MarsMiscGeo.cc 0.00% 28 Missing ⚠️
src/multio/datamod/MarsMiscGeo.h 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #183      +/-   ##
===========================================
- Coverage    56.60%   56.41%   -0.19%     
===========================================
  Files          316      317       +1     
  Lines        20489    20472      -17     
  Branches      1569     1586      +17     
===========================================
- Hits         11597    11550      -47     
- Misses        8892     8922      +30     

☔ 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.

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.

3 participants