Skip to content

grib1-to-grib2: Add option to set type=fc,strem=oper as a control for… #257

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.

Sign up for GitHub

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

Merged
dsarmany merged 4 commits into from
Apr 9, 2026
Merged

grib1-to-grib2: Add option to set type=fc,strem=oper as a control for… #257

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d84b006
grib1-to-grib2: Add option to set type=fc,strem=oper as a control for…
dsarmany Mar 31, 2026
9084813
Fix logic for checking if forecast should be a control forecast
dsarmany Apr 8, 2026
152a0f1
Improve error message
dsarmany Apr 8, 2026
08f2ff0
Improve `--control` option description
dsarmany Apr 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 21 additions & 2 deletions src/multio/tools/grib1-to-grib2.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -823,6 +823,8 @@ class Grib1ToGrib2 final : public multio::MultioTool {
std::optional<std::string> setModel_ = {};
bool mapWMOUnits_ = false;
bool noOutput_ = false;
bool control_ = false;

std::optional<std::reference_wrapper<const mars2mars::RuleList>> mappingRules_ = mars2mars::allRulesNoWMOMapping();
Discipline192Handling discipline192Handling_ = Discipline192Handling::LogAndIgnore;
};
Expand All @@ -836,6 +838,10 @@ Grib1ToGrib2::Grib1ToGrib2(int argc, char** argv) : multio::MultioTool{argc, arg
options_.push_back(new eckit::option::SimpleOption<bool>(
"wmo-units", "If specified params with local units will be mapped to params with WMO units"));
options_.push_back(new eckit::option::SimpleOption<bool>("verbose", "Sets verbosity to 2"));
options_.push_back(new eckit::option::SimpleOption<bool>(
"control",
"Treat input as a control forecast: sets number=0, adjusts ensemble-related keys, and may enforce "
"specific stream/type constraints"));
options_.push_back(
new eckit::option::SimpleOption<long>("verbosity",
"Verbosity level: 0 (print nothing), 1 (print mars keys per message), 2 "
Expand Down Expand Up @@ -873,8 +879,8 @@ void Grib1ToGrib2::init(const eckit::option::CmdArgs& args) {
args.get("all", all);
copyGrib2Messages_ = !all;

noOutput_ = false;
args.get("no-output", noOutput_);
args.get("control", control_);

std::string packing;
args.get("packing", packing);
Expand All @@ -886,7 +892,6 @@ void Grib1ToGrib2::init(const eckit::option::CmdArgs& args) {
throw std::runtime_error(std::string("Unsupported packing: ") + packing);
}
}

std::string model;
args.get("model", model);
if (!model.empty()) {
Expand Down Expand Up @@ -1052,6 +1057,20 @@ void Grib1ToGrib2::execute(const eckit::option::CmdArgs& args) {
misc.generatingProcessIdentifier.set(ncycle_);
}

// TODO: Move this logic into the encoder
// TODO: numberOfForecastsInEnsemble needs a default in the encoder
if (control_) {
if ((mars.stream.get() != "oper") || (mars.type.get() != "fc")) {
throw eckit::UserError(
Comment on lines +1060 to +1064
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

--control currently only affects the re-encode path. If the input message is GRIB2 and copyGrib2Messages_ is true, the message is copied verbatim and the control settings are silently ignored. Consider forcing re-encoding (or emitting an explicit error/warning) when control_ is set so the option reliably takes effect.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator Author

@dsarmany dsarmany Apr 8, 2026

Choose a reason for hiding this comment

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

This is okay and by design. We can already force re-encoding by passing the option --all.

"Setting forecast member to control is only supported for stream=oper and type=fc; got stream=" +
mars.stream.get() + ", type=" + mars.type.get(),
Here());
}
mars.number.set(0);
misc.typeOfEnsembleForecast.set(1);
misc.numberOfForecastsInEnsemble.set(51);
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

misc.numberOfForecastsInEnsemble.set(51) is a hard-coded ensemble size and will overwrite any value extracted from the input/mappings. If the input already provides numberOfForecastsInEnsemble, prefer keeping it; otherwise consider making the default configurable (or deriving it from context) to avoid emitting incorrect metadata for non-51-member ensembles.

Suggested change
misc.numberOfForecastsInEnsemble.set(51);
if (!misc.numberOfForecastsInEnsemble.isSet()) {
misc.numberOfForecastsInEnsemble.set(51);
}

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator Author

@dsarmany dsarmany Apr 8, 2026

Choose a reason for hiding this comment

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

This is a special case when the option --control is passed, which is meant to be used only if we want to override the non-ensemble input message. This key will be inferred from the input message and passed correctly when this option is not used.

}

if (verbosity_ > 2) {
util::PrintStream ps{std::cout};
ps << "Extracted mars dict:" << std::endl;
Expand Down
Loading