[ZFDB] FDB-721 Gribjump extractor by tbkr · Pull Request #335 · ecmwf/fdb

tbkr · 2026-08-18T09:21:58Z

Description

Implementation of GribJump backend with a CustomStoreBuilder.

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 FDB 🚧📖🌦️🌈
https://sites.ecmwf.int/docs/fdb/pull-requests/PR-335

Copilot

Pull request overview

Implements GribJump-backed partial GRIB extraction and hierarchical Zarr store construction, alongside extractor API, packaging, documentation, and test restructuring.

Changes:

Adds GribJump value selection and implicit-dimension chunking.
Introduces CustomStoreBuilder and dimension metadata.
Refactors extractor ownership, bindings, packaging, tests, and documentation.

Reviewed changes

Copilot reviewed 110 out of 111 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
`src/chunked_data_view/extractors/gribjump/GribJumpExtractor.cc`	Implements GribJump extraction.
`src/chunked_data_view/extractors/gribjump/GribJumpExtractor.h`	Declares GribJump extractor.
`src/chunked_data_view/extractors/gribjump/GribJumpExtractorDefinition.cc`	Builds configured extractors.
`src/chunked_data_view/extractors/gribjump/GribJumpExtractorDefinition.h`	Defines extractor factory.
`src/chunked_data_view/extractors/grib/GribExtractor.cc`	Refactors GRIB retrieval.
`src/chunked_data_view/extractors/grib/GribExtractor.h`	Updates GRIB extractor API.
`src/chunked_data_view/extractors/grib/GribExtractorDefinition.cc`	Adds GRIB factory.
`src/chunked_data_view/extractors/grib/GribExtractorDefinition.h`	Declares GRIB factory.
`src/chunked_data_view/ChunkedDataViewBuilder.cc`	Builds per-part extractors.
`src/chunked_data_view/ChunkedDataViewImpl.cc`	Supports implicit-axis chunks.
`src/chunked_data_view/ChunkedDataViewImpl.h`	Updates extractor ownership.
`src/chunked_data_view/ViewPart.cc`	Removes stored layout.
`src/chunked_data_view/ViewPart.h`	Revises part metadata.
`src/chunked_data_view/RequestManipulation.cc`	Adds sample-request reduction.
`src/chunked_data_view/RequestManipulation.h`	Exposes request helper.
`src/chunked_data_view/Axis.cc`	Improves lookup errors.
`src/chunked_data_view/Axis.h`	Clarifies growing chunks.
`src/chunked_data_view/Fdb.cc`	Updates FDB integration.
`src/chunked_data_view/ListIterator.cc`	Returns field locations.
`src/chunked_data_view/LibChunkedDataView.cc`	Updates licensing.
`src/chunked_data_view/mapping/AxisMapper.cc`	Updates licensing.
`src/chunked_data_view/mapping/AxisMapper.h`	Updates licensing.
`src/chunked_data_view/mapping/IndexMapper.cc`	Updates licensing.
`src/chunked_data_view/mapping/IndexMapper.h`	Updates licensing.
`src/chunked_data_view/include/chunked_data_view/AxisDefinition.h`	Adds dimension names.
`src/chunked_data_view/include/chunked_data_view/ChunkedDataView.h`	Updates licensing.
`src/chunked_data_view/include/chunked_data_view/ChunkedDataViewBuilder.h`	Adds extractor definitions.
`src/chunked_data_view/include/chunked_data_view/DataLayout.h`	Adds per-chunk size.
`src/chunked_data_view/include/chunked_data_view/Extractor.h`	Defines extractor configurations.
`src/chunked_data_view/include/chunked_data_view/Fdb.h`	Updates licensing.
`src/chunked_data_view/include/chunked_data_view/LibChunkedDataView.h`	Updates licensing.
`src/chunked_data_view/include/chunked_data_view/ListIterator.h`	Adds location-backed entries.
`src/chunked_data_view/include/chunked_data_view/Types.h`	Adds chunk bounding-box alias.
`src/chunked_data_view/include/chunked_data_view/exception/AxisMapperException.h`	Updates licensing.
`src/chunked_data_view/include/chunked_data_view/exception/BoundingBoxException.h`	Updates licensing.
`src/chunked_data_view/include/chunked_data_view/exception/GribExtractorException.h`	Updates licensing.
`src/chunked_data_view/include/chunked_data_view/exception/GribJumpExtractorException.h`	Adds GribJump exception.
`src/chunked_data_view/include/chunked_data_view/exception/RequestManipulationException.h`	Updates licensing.
`src/chunked_data_view/include/chunked_data_view/exception/UnknownExtractorException.h`	Updates licensing.
`src/chunked_data_view/exception/AxisMapperException.cc`	Updates licensing.
`src/chunked_data_view/exception/BoundingBoxException.cc`	Updates licensing.
`src/chunked_data_view/exception/GribExtractorException.cc`	Updates licensing.
`src/chunked_data_view/exception/GribJumpExtractorException.cc`	Implements new exception.
`src/chunked_data_view/exception/RequestManipulationException.cc`	Updates licensing.
`src/chunked_data_view/exception/UnknownExtractorException.cc`	Updates licensing.
`src/chunked_data_view/CMakeLists.txt`	Builds and links GribJump.
`src/chunked_data_view_bindings/bindings.cc`	Exposes new Python API.
`src/chunked_data_view_bindings/CMakeLists.txt`	Links binding dependencies.
`src/chunked_data_view_bindings/__init__.py`	Initializes packaged bindings.
`src/pychunked_data_view/chunked_data_view.py`	Adds extractor configuration wrappers.
`src/pychunked_data_view/__init__.py`	Exports value requests.
`src/pychunked_data_view/exceptions.py`	Updates licensing.
`src/z3fdb/custom_store_builder.py`	Adds hierarchical store builder.
`src/z3fdb/simple_store_builder.py`	Accepts configurable extractors.
`src/z3fdb/__init__.py`	Exports custom builder.
`src/z3fdb/_internal/__init__.py`	Updates licensing.
`src/z3fdb/z3fdb_error.py`	Updates licensing.
`src/CMakeLists.txt`	Reworks Python staging and stubs.
`tests/chunked_data_view/test_axis.cc`	Updates licensing.
`tests/chunked_data_view/test_bounding_box.cc`	Updates licensing.
`tests/chunked_data_view/test_index_mapper.cc`	Updates licensing.
`tests/chunked_data_view/test_mock_helpers.h`	Adapts extractor mocks.
`tests/chunked_data_view/test_request_manipulation_bounding_box.cc`	Updates licensing.
`tests/chunked_data_view/test_view_individual_chunking.cc`	Uses extractor definitions.
`tests/conftest.py`	Adds divergence/vorticity fixtures.
`tests/pychunked_data_view/CMakeLists.txt`	Revises Python test registration.
`tests/pychunked_data_view/fdb/test_chunked_data_view_fdb.py`	Updates GRIB builder tests.
`tests/pychunked_data_view/gribjump/test_chunked_data_view_gribjump.py`	Tests value requests.
`tests/pychunked_data_view/test_chunked_data_view_errors.py`	Tests Python errors.
`tests/pychunked_data_view/test_mars_selection.py`	Updates licensing.
`tests/z3fdb/CMakeLists.txt`	Reorganizes Z3FDB tests.
`tests/z3fdb/integration/test_gribjump_extractor.py`	Tests GribJump integration.
`tests/z3fdb/integration/test_mixed_extractors.py`	Tests mixed backends.
`tests/z3fdb/permutation_tests/test_axis_definition_ordering.py`	Tests axis ordering.
`tests/z3fdb/permutation_tests/test_axis_definition_permutations.py`	Tests axis permutations.
`tests/z3fdb/permutation_tests/test_scrambled_request_ordering.py`	Tests request ordering.
`tests/z3fdb/permutation_tests/test_store_missing_values.py`	Updates missing-value tests.
`tests/z3fdb/test_store_v3.py`	Removes superseded tests.
`tests/z3fdb/test_store_v3_pattern.py`	Removes superseded tests.
`tests/z3fdb/test_store_v3_random_axis.py`	Removes superseded tests.
`tests/z3fdb/unit/test_store_v3.py`	Adds store and chunk-list tests.
`tests/z3fdb/unit/test_store_v3_errors.py`	Updates extractor API usage.
`tests/z3fdb/unit/test_u_v_vo_d_retrieval.py`	Tests parameter derivation behavior.
`cmake/z3fdb_setup.py.in`	Packages generated stubs.
`cmake/z3fdb_setup.cfg.in`	Updates licensing.
`requirements.txt`	Pins Zarr and adds stubgen.
`pytest.toml`	Configures import mode.
`ruff.toml`	Adds Python lint settings.
`.github/workflows/z3fdb.yml`	Installs build requirements.
`docs/conf.py`	Updates documentation branding.
`docs/fdb/index.rst`	Simplifies FDB title.
`docs/index.rst`	Adds indices page.
`docs/indices_and_tables.rst`	Adds index links.
`docs/pyfdb/conftest.py`	Updates licensing.
`docs/z3fdb/api.rst`	Documents extractor API.
`docs/z3fdb/architecture.rst`	Updates licensing.
`docs/z3fdb/conf.py`	Updates licensing.
`docs/z3fdb/dimension_mapping.rst`	Updates chunking syntax.
`docs/z3fdb/getting_started.rst`	Uses new extractor API.
`docs/z3fdb/index.rst`	Reorganizes documentation.
`docs/z3fdb/installation.rst`	Adds installation guide.
`docs/z3fdb/technical_insights.rst`	Removes monolithic guide.
`docs/z3fdb/technical_insights/buffer_layout.rst`	Documents buffer layout.
`docs/z3fdb/technical_insights/chunk_access.rst`	Documents chunk access.
`docs/z3fdb/technical_insights/dev_setup.rst`	Adds development guide.
`docs/z3fdb/technical_insights/extractor.rst`	Documents extractor internals.

Suppressed comments (2)

src/chunked_data_view/extractors/grib/GribExtractor.cc:122

FieldLocation::dataHandle() returns an owning raw pointer (the previous iterator wrapped it in unique_ptr). Keeping it in auto leaks one handle for every GRIB field read, so repeated chunk access steadily consumes resources. Restore RAII ownership here.
src/chunked_data_view/extractors/gribjump/GribJumpExtractor.cc:202
Every field in every chunk prints its storage fragment directly to stdout. Large reads will emit thousands of lines, interfere with callers' output, and add avoidable I/O overhead. Remove this debugging output (and the adjacent unused variable).

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+std::unique_ptr<Extractor> GribJumpExtractorDefinition::buildExtractor(const metkit::mars::MarsRequest& request) const {
+    if (gribjumpConfig) {
+        ::setenv("GRIBJUMP_CONFIG_FILE", gribjumpConfig->c_str(), /*overwrite=*/1);
+    }


…im_names support Introduce the GribJump-backed extractor alongside a suite of related refactors that improve the extractor ownership model, Python API surface, and zarr metadata output. Add `GribJumpExtractor` / `ExtractorType.GribJump` as a second extractor implementation. It uses the GribJump library to retrieve a configurable sub-range of grid-point values without a full GRIB decode, making it efficient for spatial sub-region access. Field-level chunking of the implicit grid-point axis is controlled by `field_chunking` (default `WholeAxisChunking`). Mixing `GribExtractor` and `GribJumpExtractor` parts is valid only when `GribJumpExtractor` uses `WholeAxisChunking`; all other `field_chunking` variants raise at build time. - Replace `std::shared_ptr<Extractor>` with `std::unique_ptr<Extractor>`. Each `ViewPart` owns its extractor exclusively; sharing between parts is no longer supported. - Introduce the `ExtractorDefinition` factory pattern: `addPart` records a lightweight config object; `build()` calls `buildExtractor(request)` once per part, deferring FDB/GribJump initialisation to build time. - `ChunkedDataViewBuilder` is now non-copyable (copy constructor and assignment operator explicitly deleted) because it stores `unique_ptr<ExtractorDefinition>`. - `ListIteratorWrapperImpl::next()` now returns `ListElement` directly. Add `CustomStoreBuilder` alongside `SimpleStoreBuilder`. It accepts an arbitrary path-based hierarchy of `ChunkedDataViewBuilder` instances and materialises them into a nested `FdbZarrGroup` / `FdbZarrArray` tree, enabling multi-array zarr stores from a single builder. - Add `std::optional<std::string> name` to the `AxisDefinition` C++ struct and expose it via pybind11 `def_readwrite`. - Track dimension names in the Python layer only: `ChunkedDataViewBuilder` derives `dim_names` from `axis.name` (explicit) or `"_".join(axis.keys)` (auto) on the first `add_part` call; the implicit grid-point axis is always named `"values"`. - `FdbSource` accepts `dim_names` and forwards them to `DotZarrArrayJson.dimension_names`, so zarr v3 array metadata carries labelled dimensions consumable by xarray. - Rename `Chunking.FixedSizeChunk.chunkShape` → `chunk_shape`. - Remove unused store flags. - `extend_on_axis` now supports extension on any axis index, not just the first. - Restructure API tests: move `user_tests/` and `zarr_interface_conformity/` under a new `interface/` directory; update `CMakeLists.txt` accordingly. - Add `test_metadata_mapping.py`: verifies explicit and auto-derived axis names appear in zarr array metadata. - Add `test_xarray_access.py`: exercises xarray integration via `dimension_names` (single-part, fixed-size chunking, multi-part SFC+PL, named-dim reduction). - Update extractor docs: ownership model (`unique_ptr`, non-copyable), `ExtractorDefinition` factory, `GribJumpExtractor` value ranges and configuration.

Follow-up on the FDB-721 review. Groups ten commits covering build decoupling, correctness, the Python/C++ seam, the store builders, tests, docs and wheel packaging. Build - GribJump moves behind ENABLE_ZARR_GRIBJUMP_EXTRACTOR, default off. fdb cannot link an installed gribjump, because gribjump depends on fdb5, so the extractor only builds inside a bundle. Availability is exposed as has_gribjump_extractor and dependent tests skip without it. - The CD build config sets the flag off explicitly. Correctness - DataHandle from ListIterator is owned, it leaked on every field. - Parts are validated against each other at build time. Mixing a Grib and a GribJump part with differing value layouts returned wrong data instead of failing. - Extractor calls into fdb and gribjump are serialised per extractor. - Assertions replace unchecked indexing and string conversion. Python/C++ seam - add_part copies the extractor definition, so one definition object can be reused across parts and builders. - The builder fills its own fdb config into definitions that lack one. - Extractor failures surface as catchable exception types. Store builders - extend_on_axis and fill_missing_value reject unregistered paths. A typo used to create a second empty array and fail much later. - SimpleStoreBuilder delegates to CustomStoreBuilder instead of duplicating it. Tests - GribJump tests move into their own folder behind the skip marker. - Bitmap and ramp fixtures assert their own preconditions, two tests were passing without checking what they claimed to check. Docs - New Reference part, and a page contrasting the two extractor backends with their constraints and availability. Packaging - Stub generation and the wheel build run through uvx, so a runner shipping only an interpreter can build. Dependencies are split into z3fdb-requirements.txt, z3fdb-build-requirements.txt and z3fdb-testing-requirements.txt.

codecov-commenter · 2026-09-02T08:09:45Z

Codecov Report

❌ Patch coverage is 76.01626% with 59 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.54%. Comparing base (bdbdbd1) to head (76b82d0).

Files with missing lines	Patch %	Lines
...chunked_data_view/extractors/grib/GribExtractor.cc	0.00%	23 Missing ⚠️
...ta_view/extractors/grib/GribExtractorDefinition.cc	0.00%	9 Missing ⚠️
...extractors/gribjump/GribJumpExtractorDefinition.cc	0.00%	8 Missing ⚠️
src/chunked_data_view/ChunkedDataViewBuilder.cc	80.64%	6 Missing ⚠️
src/chunked_data_view/ListIterator.cc	0.00%	3 Missing ⚠️
..._data_view/exception/GribJumpExtractorException.cc	0.00%	3 Missing ⚠️
tests/chunked_data_view/test_mock_helpers.h	87.50%	3 Missing ⚠️
src/chunked_data_view/ChunkedDataViewImpl.cc	91.66%	1 Missing ⚠️
...ata_view/extractors/grib/GribExtractorDefinition.h	0.00%	1 Missing ⚠️
.../extractors/gribjump/GribJumpExtractorDefinition.h	0.00%	1 Missing ⚠️
... and 1 more

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #335      +/-   ##
===========================================
+ Coverage    77.52%   77.54%   +0.01%     
===========================================
  Files          411      417       +6     
  Lines        27612    27768     +156     
  Branches      2776     2782       +6     
===========================================
+ Hits         21407    21533     +126     
- Misses        6205     6235      +30

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

jharwood0 · 2026-09-02T14:51:48Z

+        gj_requests.emplace_back(location_uri.path(), location_uri.scheme(), fieldOffset, location_uri.host(),
+                                 location_uri.port() > 0 ? location_uri.port() : 0,
+                                 std::vector<gribjump::Range>{chunkRange});


I was only able to run this when GRIBJUMP_IGNORE_GRID is set, as gribjump seems to reject a request with empty hash - which is the default?
https://github.com/ecmwf/gribjump/blob/develop/src/gribjump/ExtractionData.h#L76

Would it make sense to pull the hash out of the grib message, and provide it to gribjump?

gridHash_ = msg.getString("md5GridSection");

tbkr requested a balanced review from Copilot August 18, 2026 09:22

Copilot started reviewing on behalf of tbkr August 18, 2026 09:22 View session

tbkr force-pushed the feature/zfdb/fdb-721-gribjump-extractor branch from b24127c to 93a9748 Compare August 18, 2026 09:25

Copilot AI reviewed Aug 18, 2026

View reviewed changes

tbkr force-pushed the feature/zfdb/fdb-721-gribjump-extractor branch 15 times, most recently from 50495e0 to e70310c Compare August 20, 2026 09:21

caraghbiner self-requested a review August 20, 2026 10:13

tbkr force-pushed the feature/zfdb/fdb-721-gribjump-extractor branch 10 times, most recently from 585208d to 5b19b29 Compare August 24, 2026 15:51

tbkr force-pushed the feature/zfdb/fdb-721-gribjump-extractor branch 2 times, most recently from 7798054 to ce36c4f Compare August 25, 2026 08:19

tbkr requested a review from jharwood0 August 25, 2026 08:47

tbkr marked this pull request as ready for review August 25, 2026 08:47

tbkr force-pushed the feature/zfdb/fdb-721-gribjump-extractor branch from ce36c4f to cd0a573 Compare August 25, 2026 08:50

tbkr force-pushed the feature/zfdb/fdb-721-gribjump-extractor branch from cd0a573 to 8fcfcde Compare September 1, 2026 12:19

tbkr added 2 commits September 2, 2026 09:49

tbkr force-pushed the feature/zfdb/fdb-721-gribjump-extractor branch from 8fcfcde to 76b82d0 Compare September 2, 2026 07:49

jharwood0 reviewed Sep 2, 2026

View reviewed changes

Navigation Menu

Uh oh!

[ZFDB] FDB-721 Gribjump extractor - #335

[ZFDB] FDB-721 Gribjump extractor#335
tbkr wants to merge 2 commits into
developfrom
feature/zfdb/fdb-721-gribjump-extractor

tbkr commented Aug 18, 2026 •

edited by github-actions Bot

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Sep 2, 2026

Uh oh!

jharwood0 Sep 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

tbkr commented Aug 18, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Contributor Declaration

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Sep 2, 2026

Codecov Report

Uh oh!

jharwood0 Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tbkr commented Aug 18, 2026 •

edited by github-actions Bot

Loading