Fixes and metadata changes to make ocean work with mtg2 keys#277
Conversation
68fec90 to
3125744
Compare
| const auto mars = dm::dumpRecord<eckit::LocalConfiguration>(marsRec); | ||
| const auto misc = dm::dumpRecord<eckit::LocalConfiguration>(miscRec); | ||
|
|
||
| const auto misc = dm::dumpUnscopedRecord<eckit::LocalConfiguration>(miscRec); |
There was a problem hiding this comment.
This change is also provided by a separate hotfix:
#275
Without that change, misc keys are not passed through.
| message::Metadata& md = msg.modifyMetadata(); | ||
| md.set("missingValue", missingValue_); | ||
| md.set("bitmapPresent", true); | ||
| md.set("misc-missingValue", missingValue_); |
There was a problem hiding this comment.
This lines were only used by NEMO throug the IO server so far.
As the metadata has changed in iom.F90, this metadata is now consistent with the other actions.
|
|
||
| std::ostringstream oss; | ||
| oss << rootPath_ << msg.metadata().get<std::int64_t>("level") << "::" << paramOrId | ||
| oss << rootPath_ << msg.metadata().get<std::int64_t>("levelist") << "::" << paramOrId |
There was a problem hiding this comment.
The key "level" is not used in mtg2 anymore. "levelist" is the appropriate mars key.
| return disableSquashing_; | ||
| } | ||
| const std::vector<std::pair<std::string, std::string>>& StatisticsOptions::setMetadata() const { | ||
| const message::Metadata& StatisticsOptions::setMetadata() const { |
There was a problem hiding this comment.
Changing from vector<pair<string,string>> to a proper Metadata was necessary to maintain the underlying type, especially for the key "misc-timeIncrementInSeconds".
The set-metadata is currently used to "fix" the "misc-timeIncrementInSeconds" key after the monthly statistics for ocean (which is not using stattype....).
|
|
||
| bool OperationWindow::gtLowerBound(const eckit::DateTime& dt, bool throw_error) const { | ||
| if (throw_error && creationPoint_ >= dt) { | ||
| if (throw_error && startPoint_ >= dt) { |
There was a problem hiding this comment.
These changes are important and correct.
The window ranges from startPoint to endPoint.
The creationPoint can be the same as startPoint, but is more likely just to be within the window.
For ocean, no initial condition is send, after the first message arrives with time 1h. This was not passing through here.
| if (opname != "instant") { | ||
| if (currentLoop == 1) { | ||
| const std::int64_t timespan = ts.win().currPointInHours() - ts.win().creationPointInHours(); | ||
| const std::int64_t timespan |
There was a problem hiding this comment.
The probably less intuitive change - the time span computation depends whether the window is viewed from the starting point or the creation point.
For monthly output, the decision is made to use the creation point - notably to explicitly avoid having "full" months if the forecast started in the middle of a month.
For daily statistics the starting point is of interest, i.e. time 0000 of a day instead of 0100.
| LOG_DEBUG_LIB(LibMultio) << opt.logPrefix() << " *** Load restart files" << std::endl; | ||
| } | ||
|
|
||
| const PeriodUpdater& TemporalStatistics::periodUpdater() const { |
There was a problem hiding this comment.
Allow accessing the period updater to get information on the window size categorization.
| std::ostringstream os; | ||
| os << "hour"; | ||
| return os.str(); | ||
| return "hour"; |
There was a problem hiding this comment.
Removing the ostringstream is obvious.
Further refactorings should consider using a proper enum.
| return conf.getStringVector("hash-keys"); | ||
| } | ||
| return std::vector<std::string>{"category", "name", "level"}; | ||
| return std::vector<std::string>{"param", "levtype", "levelist"}; |
There was a problem hiding this comment.
Very important change to go fully on MARS based actions.
The transport is currently only used by nemo, the keys here have been introduced with NEMO as first use case.
I could have simply adjusted the hash keys via plan configuration, however the usage of MARS keys is more general for the other future usecases.
Once nemo is migratrated to mtg2, "category", "name" and "level" are not used anymore.
| throw multio::transport::TransportException(os.str(), Here()); | ||
| } | ||
| return searchHashKey->second; | ||
| return std::hash<message::MetadataValue>{}(searchHashKey->second); |
There was a problem hiding this comment.
Use std::hash directly instead of string conversion....
| /// This file contains daily and monthly statistics. | ||
| /// This primarily tests current behaviour for ocean output. | ||
| /// | ||
| /// Test param: 262100 -> 263100 (average) |
| "options": { | ||
| "set-metadata": { | ||
| "stream": "clmn", | ||
| "misc-timeIncrementInSeconds": 86400 |
There was a problem hiding this comment.
timeIncrementInSeconds may need to set to 0 to explicitly disable it for encoding
Description
All necessary changes to get NEMO running with updated mtg2 metadata.
As NEMO is the only one using the IO server right now, metadata in some other actions (statistics-mtg2, mask, aggregation, transport, encode-mtg2) have been made MTG2 compliant.
The necessary NEMO changes can be found here: https://git.ecmwf.int/users/mapg/repos/ifs-source/commits?until=refs%2Fheads%2Ffeature%2FCY49R3_v4_20260126_ocean_mtg2
The changes require additional mars2grib changes for encoding: ecmwf/metkit#213
The relevant multio plans for climate-dt (as reference) can be found here: https://git.ecmwf.int/users/ecm4053/repos/multio_plans/pull-requests/8/overview
Detailed changes with explanation will be commented directly in the code (TBD).
Expected difference in encoding for climateDT:
CI fails unless the metkit PR is merged
Contributor Declaration
By opening this pull request, I affirm the following:
🌈🌦️📖🚧 Documentation 🚧📖🌦️🌈
https://sites.ecmwf.int/docs/dev-section/multio/pull-requests/PR-277