Skip to content

convert-mars-request: Flatten only rule keys and keep non-field keys - #285

Open
dsarmany wants to merge 1 commit into
developfrom
feature/convert-mars-request-regrouping
Open

dsarmany wants to merge 1 commit into
developfrom
feature/convert-mars-request-regrouping

Conversation

@dsarmany

@dsarmany dsarmany commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Converting a request used to flatten it into one request per field, which ran out of memory on large requests (~5M fields). The compaction afterwards split levelist=all requests per param, lost keys that are not hypercube axes (grid, area, target, ...) whenever a group was sparse, and printed groups in key-set order rather than request order.

  • Flatten only the keys read by the mars2mars rules; all other keys keep their lists of values
  • Compact each group over the keys that vary within it, on a copy of its first request, without relying on MarsRequest::count()
  • Order the output by the position of the first field in the input
  • Pass post-processing, sink and unknown keys through without validating or expanding them, inherited like MARS does

Add a regression test for convert-mars-request.

Description

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.

📋 Metkit Documentation 📋
https://sites.ecmwf.int/docs/metkit/pull-requests/PR-285

@dsarmany
dsarmany requested a review from danovaro September 25, 2026 13:31
@codecov-commenter

codecov-commenter commented Sep 25, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 42.80443% with 155 lines in your changes missing coverage. Please review.
✅ Project coverage is 50.86%. Comparing base (b7bc7a1) to head (54fb723).

Files with missing lines Patch % Lines
src/tools/ParseRequest.cc 1.37% 143 Missing ⚠️
src/metkit/mars/TypeParam.cc 89.87% 8 Missing ⚠️
src/metkit/mars/MarsLanguage.cc 0.00% 2 Missing ⚠️
src/metkit/mars2mars/mappings/mappings.h 50.00% 1 Missing ⚠️
src/metkit/mars2mars/mappings/rules/timespan.h 83.33% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #285      +/-   ##
===========================================
- Coverage    50.96%   50.86%   -0.10%     
===========================================
  Files          481      481              
  Lines        22812    23035     +223     
  Branches      1569     1620      +51     
===========================================
+ Hits         11625    11717      +92     
- Misses       11187    11318     +131     

☔ View full report in Codecov by Harness.
📢 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.

@dsarmany
dsarmany force-pushed the feature/convert-mars-request-regrouping branch from 736a85a to 60cd68c Compare September 26, 2026 11:16
@danovaro
danovaro force-pushed the feature/convert-mars-request-regrouping branch 2 times, most recently from 15cba60 to 269d99f Compare October 5, 2026 08:10
Converting a request used to flatten it into one request per field,
which ran out of memory on large requests (~5M fields).

- Flatten only the keys read by the mars2mars rules
- Compact each group over the keys that vary within it without relying
  on MarsRequest::count()
- Order the output by the position of the first field in the input
- Pass post-processing, sink and unknown keys through without
  validating or expanding them, inherited like MARS does
- Add a regression test for convert-mars-request.

Add packing and interpolation aliases to the language

mars2mars: Do not fail on step ranges, missing param or missing step

- Read step as a string before trying long, so that a step range such
  as 0-24 in a MarsRequest reaches the range conversion instead of
  failing to parse as long
- Write step to a MarsRequest as a plain number (24, not 24h)
- Leave requests without param unchanged (e.g. type=tf tracks)
- Skip the timespan fix when there is no timespan, as for products
  indexed by fcmonth rather than step
- Add these cases to the convert-mars-request regression test

mars2mars: Convert stream enwh to enfh

Wave hindcasts are encoded as enfh, similar to how 'wave' maps to 'oper'
and 'waef' to 'enfo'. Add a test for the three wave stream conversions.

mars2mars: Convert CY50r1 wave streams to their CY50r2 equivalents

Only wave, waef and enwh were converted before.

- BC and long-window data: scwv, dcwv, lwwv, ewda, ewla, fsow
- Ensemble, extended range and hindcasts: weef, weeh, ewho, weov, ewhc
- Hindcast statistics: wehs, wees
- Monthly means and climatology: wamo, wamd, ewmm, ewmo, dacw

The legacy seasonal and monthly forecast wave streams, and the wave
streams without a clear counterpart (mawv, wvhc, wavm), are unchanged.

TypeParam: Read bare param numbers from the table of the request context

A param number without table was read as a table-128 paramId whenever
no rule of the context listed it, even when the context held no such
param. For example, param=229 in stream=wave became 229 (iews) instead
of 140229 (swh), and param=246 at levtype=sfc became 246 (clwc) instead
of 228246 (100u). MARS reads such numbers from the table that holds them
in the request context.

For each fully specified context (class, levtype, stream, type), record
the numbers that are not in table-128 but are in at least one other
table. If they appear in more than one table, use table 228. These
resolutions are consulted before the existing lookup, so explicit tables
(246.128), table-128 params of the context and shortnames are
unaffected. They are cached in params.bin, whose version is bumped to 2.

convert-mars-request: Validate post-processing keys known to the language

Post-processing and sink keys were passed through verbatim, so their
values were neither validated nor normalised. Now that the language
accepts the usual spellings (e.g. second_order, nearest_neighbour), pass
through only the keys unknown to the language of the verb (e.g.
password), and let the expansion validate and normalise all others.

Add MarsLanguage::isKeyword() to tell keywords and their aliases from
unknown keys.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@danovaro
danovaro force-pushed the feature/convert-mars-request-regrouping branch from 269d99f to 54fb723 Compare October 5, 2026 10:23
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