Added config reader data source #6

cducher · 2025-02-21T22:48:05Z

This data reader uses a configuration to provide data to the emulator. It supports five generation methods (see configuration on README for examples)
These are examples of fields it can generate (grid is N80):

codecov-commenter · 2025-02-21T22:55:26Z

Codecov Report

Attention: Patch coverage is 0% with 433 lines in your changes missing coverage. Please review.

Project coverage is 24.21%. Comparing base (42b7ea9) to head (32be034).

Files with missing lines	Patch %	Lines
src/nwp_emulator/config_reader.cc	0.00%	237 Missing ⚠️
src/nwp_emulator/config_reader_funcs.cc	0.00%	170 Missing ⚠️
src/nwp_emulator/config_reader.h	0.00%	16 Missing ⚠️
src/nwp_emulator/nwp_data_provider.cc	0.00%	8 Missing ⚠️
src/nwp_emulator/base_data_reader.h	0.00%	2 Missing ⚠️

Additional details and impacted files

@@                   Coverage Diff                    @@
##           feature/nwp-emulator       #6      +/-   ##
========================================================
- Coverage                 29.46%   24.21%   -5.25%     
========================================================
  Files                        49       52       +3     
  Lines                      1972     2399     +427     
  Branches                    138      249     +111     
========================================================
  Hits                        581      581              
- Misses                     1391     1818     +427

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

src/nwp_emulator/config_reader_funcs.cc

antons-it

thanks, nice work! this will make the development/testing of plugins much easier

antons-it · 2025-03-14T09:22:52Z

src/nwp_emulator/README.md

+
+Here is an example of YAML configuration file you can use to generate model data, which demonstrate the options that each generation function accept:
+
+```yaml


Nice and useful configuration example! we could consider having a separate entry for "functions" (at the same level of "fields") where named functions are defined. Then these named functions could be recalled within the fields/levels - to avoid duplications. But this can definitely be another PR

dsarmany · 2025-03-14T11:54:30Z

src/nwp_emulator/README.md

+      apply:
+        vortex_rollup:
+          time_variation: 1.1
+    u: # no specified levtype means not surface


Is getting u,v from model-level grib output supported?

dsarmany · 2025-03-14T11:56:57Z

src/nwp_emulator/config_reader.h

+     *
+     * @return The rectangular area of the map or a null pointer if the whole globe should be considered.
+     */
+    FocusArea* setFocusArea(const eckit::LocalConfiguration& options);


Return std::unique_ptr<FocusArea> instead of a raw pointer.

dsarmany · 2025-03-14T11:59:55Z

src/nwp_emulator/config_reader.cc

+    }
+}
+
+ConfigReader::~ConfigReader() {}


You can remove this completely.

dsarmany · 2025-03-14T12:00:25Z

src/nwp_emulator/config_reader.h

+    ConfigReader(const eckit::PathName& inputPath, size_t rank, size_t root, int stepCountLimit = 100);
+
+    /// Makes sure that dynamically allocated memory is freed.
+    ~ConfigReader();


You can remove this completely.

dsarmany · 2025-03-14T15:26:32Z

src/nwp_emulator/config_reader.cc

+        fieldConfigStr += ".levels." + levelKey;
+    }
+    readerConfig_.get(fieldConfigStr, fieldConfig);
+    for (const std::string& func : fieldConfig.keys()) {


An elegant way of doing this would be to create a lookup table of std::string -> std::function .

std::map<std::string, std::function<>> myLookUpTable

dsarmany · 2025-03-14T15:53:12Z

src/nwp_emulator/config_reader_funcs.cc

+        translation[1] *= step_;
+    }
+    std::vector<double> areaCoords;
+    FocusArea* focusArea = nullptr;


Then you don't have to do this.

dsarmany · 2025-03-14T15:54:52Z

src/nwp_emulator/config_reader_funcs.cc

+        }
+        focusArea = new FocusArea{west, east, south, north};
+    }
+    return focusArea;


Then you can do just focusArea = std::make_unique<FocusArea>(west, east, south, north).

dsarmany · 2025-03-14T16:03:58Z

src/nwp_emulator/config_reader_funcs.cc

+
+    for (size_t i_pt = 0; i_pt < values.size(); i_pt++) {
+        atlas::PointLonLat p{lonlat(i_pt, atlas::LON), lonlat(i_pt, atlas::LAT)};
+        if (focusArea != nullptr && !focusArea->contains(p)) {


if (focusArea && ...)

dsarmany · 2025-03-14T16:08:03Z

src/nwp_emulator/nwp_data_provider.cc

@@ -22,7 +22,9 @@ NWPDataProvider::NWPDataProvider(const DataSourceType& sourceType, const eckit::
            dataReader = new GRIBFileReader(inputPath, rank, root);
            break;
        case DataSourceType::CONFIG:
-            eckit::Log::info() << "Config source type has not been implemented yet" << std::endl;
+            eckit::Log::info() << "Emulator will use config as source type from" << inputPath << std::endl;
+            dataReader = new ConfigReader(inputPath, rank, root);


The same here too.

dsarmany · 2025-03-14T16:16:19Z

src/nwp_emulator/nwp_data_provider.cc

@@ -71,6 +73,9 @@ NWPDataProvider::NWPDataProvider(const DataSourceType& sourceType, const eckit::
    atlas::grid::Distribution distribution(grid, atlas::util::Config("type", grid.partitioner().getString("type")) |
                                                     atlas::util::Config("bands", nprocs_));
    fs_ = atlas::functionspace::StructuredColumns(grid, distribution, atlas::util::Config("levels", nlevels));
+    if (sourceType_ == DataSourceType::CONFIG) {
+        dynamic_cast<ConfigReader*>(dataReader)->setReaderArea(fs_.lonlat());


Perhaps move this to the base class if that is what conceptually makes sense.

cducher requested review from dsarmany and antons-it February 21, 2025 22:48

cducher self-assigned this Feb 21, 2025

cducher force-pushed the feature/nwp-emulator-config-source branch from fac6af7 to 576cb67 Compare March 6, 2025 11:42

Config reader as data source feature for NWP emulator

68ec7a8

cducher force-pushed the feature/nwp-emulator-config-source branch from 576cb67 to 68ec7a8 Compare March 6, 2025 11:45

cducher changed the title ~~WIP Added config reader data source~~ Added config reader data source Mar 6, 2025

cducher marked this pull request as ready for review March 6, 2025 11:53

cducher commented Mar 7, 2025

View reviewed changes

src/nwp_emulator/config_reader_funcs.cc Outdated Show resolved Hide resolved

cducher added 2 commits March 7, 2025 11:55

Added variation flag to Step function for de/increasing step values

ed34164

Vortex rollup time variation bugfix

849f28a

antons-it approved these changes Mar 14, 2025

View reviewed changes

dsarmany requested changes Mar 14, 2025

View reviewed changes

Addressed PR feedback

32be034

cducher requested a review from dsarmany March 18, 2025 16:52

dsarmany approved these changes Mar 21, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added config reader data source #6

Added config reader data source #6

cducher commented Feb 21, 2025 •

edited

Loading

codecov-commenter commented Feb 21, 2025 •

edited

Loading

antons-it left a comment

antons-it Mar 14, 2025

dsarmany Mar 14, 2025

dsarmany Mar 14, 2025

dsarmany Mar 14, 2025

dsarmany Mar 14, 2025

dsarmany Mar 14, 2025

dsarmany Mar 14, 2025

dsarmany Mar 14, 2025

dsarmany Mar 14, 2025

dsarmany Mar 14, 2025

dsarmany Mar 14, 2025


		Here is an example of YAML configuration file you can use to generate model data, which demonstrate the options that each generation function accept:

		```yaml

Added config reader data source #6

Are you sure you want to change the base?

Added config reader data source #6

Conversation

cducher commented Feb 21, 2025 • edited Loading

codecov-commenter commented Feb 21, 2025 • edited Loading

Codecov Report

antons-it left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cducher commented Feb 21, 2025 •

edited

Loading

codecov-commenter commented Feb 21, 2025 •

edited

Loading