Repository navigation
Conversation
janickm
changed the title
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
Collaborator
Author
|
/ok to test 9b981aa |
Collaborator
Author
|
/ok to test e4bc8c3 |
janickm
commented
Oct 1, 2026
janickm
commented
Oct 1, 2026
janickm
commented
Oct 1, 2026
Collaborator
Author
|
/ok to test 19dbc59 |
Collaborator
Author
|
/ok to test 9da7388 |
janickm
commented
Oct 1, 2026
janickm
commented
Oct 1, 2026
janickm
commented
Oct 1, 2026
janickm
commented
Oct 1, 2026
janickm
commented
Oct 1, 2026
janickm
commented
Oct 1, 2026
Collaborator
Author
|
/ok to test e78d621 |
janickm
commented
Oct 1, 2026
janickm
commented
Oct 1, 2026
janickm
commented
Oct 1, 2026
Collaborator
Author
|
/ok to test d543c5e |
janickm
commented
Oct 13, 2026
This was referenced Oct 13, 2026
Merged
janickm
commented
Oct 13, 2026
janickm
commented
Oct 13, 2026
janickm
commented
Oct 13, 2026
janickm
commented
Oct 13, 2026
janickm
commented
Oct 13, 2026
janickm
commented
Oct 13, 2026
janickm
commented
Oct 13, 2026
janickm
commented
Oct 13, 2026
janickm
commented
Oct 13, 2026
janickm
commented
Oct 13, 2026
janickm
commented
Oct 13, 2026
janickm
commented
Oct 13, 2026
janickm
commented
Oct 13, 2026
| self.assertIsNone(np.testing.assert_array_equal(g_ref["x"][()], g_reload["x"][()])) | ||
|
|
||
| if __name__ == "__main__": | ||
| unittest.main() |
Collaborator
Author
There was a problem hiding this comment.
should not be required?
Collaborator
Author
There was a problem hiding this comment.
done: inlined into a small private _read_member (extractfile returns None for non-regular members only, hence the assert). Also compares record digests now, so failures don't diff large payloads
Collaborator
Author
There was a problem hiding this comment.
unsure what you changed - please revert - I meant we don't need
if __name__ == "__main__":
...
as we run through pytest + bazel anyway?
Collaborator
Author
There was a problem hiding this comment.
reverted: the original record comparisons are back (and unpack in this commit, which the typed node API commit replaces by _read_member when it rewrites the test file). Also dropped the if __name__ == "__main__" blocks, here and in the new formats_test.py
Separate the record storage of indexed tar archives (itar.py) from zarr-python, in preparation of supporting multiple zarr-python versions: - IndexedTarStore provides raw access to the records of an archive (mapping interface), stores.ZarrIndexedTarStore adapts it to zarr-python's store interface - Read local archives via positional reads (os.pread), which don't serialize reads of multiple threads, and close their file descriptors also when stores are dropped without closing them - Serve records starting within the index tail read from memory - Look up the record index decoder / encoder by index type, and share the CBOR + LZMA document encoding of the record index and the compressed consolidated metadata (encoding.py) Archives are byte-identical to archives written by ncore 19.8 (tested via the sha256 of a fixed hierarchy written by ncore 19.8). BREAKING CHANGE: IndexedTarStore is no longer a zarr-python store, wrap it via stores.ZarrIndexedTarStore to access zarr hierarchies of archives directly with zarr-python.
Access the zarr hierarchies of component stores via typed group / array wrappers instead of zarr-python APIs, as a prerequisite of supporting multiple zarr-python versions: - nodes.Group / nodes.Array: create and read (nested) groups, arrays, binary blobs and (typed) attributes; the wrapped zarr-python objects are accessible via .zarr - stores: open_directory_store, create_root_group, open_store, consolidate_store, close_store and group_store for indexed tar archives and directories with compressed consolidated metadata - Version specific operations are implemented by a zarr-python 2 backend (_zarr2.py) of a backend interface (_backend.py) - Port all components to the node API (single chunk arrays by default, blosc lz4 / bit-shuffle compression), sharing frame / point cloud path lookups Written stores are byte-identical to stores written by ncore 19.8. BREAKING CHANGE: The groups of component writers / readers (self._group) are nodes.Group instances (the wrapped zarr-python group is accessible via self._group.zarr). The stores functions consolidate_compressed_metadata / open_compressed_consolidated and the ConsolidatedCompressedMetadataStore class are replaced by consolidate_store / open_store.
Opening a group / array constructs its zarr-python object, which decodes the node's metadata on every access (~0.5 ms per frame lookup with zarr-python 2, dominating reads of small frame arrays). Component readers now cache the nodes they open (with their decoded metadata, but not their data) in a thread-safe nodes.NodeCache shared by all member groups of the component. The cache size is configurable via SequenceComponentGroupsReader(node_cache_size=...): None (default) caches all accessed nodes, N the N most recently used nodes, and 0 disables caching. With the cache, repeated frame lookups take ~2 us instead of ~470 us. Component stores are opened concurrently (hiding the latency of remote archive index reads), their hierarchies are now opened from their consolidated metadata sequentially on the calling thread: opening hierarchies is pure-Python CPU work, which concurrent threads only slow down.
Support zarr-python 3 (>= 3.1.4) next to zarr-python 2 for zarr format v2 data. The zarr-python version specific operations of nodes and stores are implemented by a zarr-python 3 backend (_zarr3.py), which is selected at import time: - zarr-python's coroutines run on an event loop owned by the calling thread (offloaded codec work runs inline), avoiding thread hand-offs to zarr-python's I/O thread per access. Forked processes create new loops, coroutines within running event loops run on zarr-python's I/O loop - a zarr-python 3 store of indexed tar archives - consolidated hierarchies are opened from the decoded compressed consolidated metadata (no node metadata reads), with node metadata ordered by parent group (zarr-python < 3.4 attaches members of a group to its first run of keys only, zarr-python#4226) Data written with zarr-python 2 and 3 is equivalent: chunk records are byte-identical, node metadata documents are equivalent JSON. zarr-python 3 rewrites node metadata on every update of a node, which the write-once records of indexed tar archives don't support. Node metadata records of archives opened for writing are now kept in memory and appended when closing the archive (parents first), independent of the zarr-python version. Test environments: python 3.11 uses zarr-python 3.1.4 (the oldest supported version), python 3.13 zarr-python 3.4, python 3.8 / 3.12 zarr-python 2. BREAKING CHANGE: node metadata records of indexed tar archives are appended when closing archives (after all chunk records), so archives are no longer byte-identical to archives written by ncore 19.8 (their records are).
Support writing and reading component stores in zarr format v3 with zarr-python 3 (zarr format v2 remains the default, readable with all zarr-python and ncore versions): - SequenceComponentGroupsWriter(zarr_format=...) / from_reader(), and stores.create_root_group(zarr_format=...), validated against the formats supported by the installed zarr-python version (nodes.check_zarr_format, nodes.SUPPORTED_ZARR_FORMATS) - data converters: --zarr-format CLI option (BaseDataConverterConfig. zarr_format) - the consolidated metadata of zarr format v3 hierarchies is stored as CBOR + zlib encoded root zarr.json (zarr.json.cbor.zlib) only, which decodes several times faster than CBOR + LZMA - binary blobs are uint8 arrays (fixed-length bytes are not part of the zarr format v3 specification), compression uses the blosc codec of zarr format v3 Component, store and converter tests run for both zarr formats. The time-trimmed zarr format v2 / v3 serializations of the test sequence (test-data-v4-trimmed 0.2) are compared by formats_test. BREAKING CHANGE: BaseDataConverterConfig requires zarr_format.
zarr-python 3's codec pipeline decodes each chunk into a temporary buffer and copies it into the output array (zarr-python#2904), and its per-read overhead dominates reads of small frame arrays (~550 us per frame read vs ~200 us with zarr-python 2). Basic selections (integers, unit-step slices, Ellipsis) of arrays of indexed tar archives opened for reading are now read directly: chunks are determined by zarr-python's indexer, read from the archive and blosc-decompressed into the output array (in place for complete chunks). Uncompressed single-chunk arrays (e.g., encoded images) are returned without decoding. Large chunks of a read (>= 2 MiB) are decoded concurrently on a thread pool, bounded to two concurrent reads per process. Supported are regular chunk grids, C order, numeric / boolean data of native byte order and fixed-length bytes, without filters, uncompressed or blosc compressed (all arrays written by ncore), all other arrays and selections are read via zarr-python. Read plans are derived per read (sub-microsecond, nothing is cached), results are writeable, missing chunks read as the fill value, and the parallel reads state is re-created in forked processes. NCORE_ZARR3_DIRECT_READS=0 reads all arrays via zarr-python. Frame reads of the test sequence take ~240 us instead of ~550 us.
Add asynchronous variants of all component reader methods reading array data (get_frame_data_async, get_frame_ray_bundle_data_async, get_frame_ray_bundle_return_data_async, get_generic_data_async, get_camera_mask_image_async, get_pc_*_async, labels get_data_async, ...), returning the same data as the synchronous methods, based on nodes.Array.read_async / read_bytes_async. Awaiting multiple reads concurrently overlaps their I/O and decompression, without blocking the running event loop: - zarr-python 3: chunks of remote indexed tar archives (asynchronous fsspec filesystems, e.g. s3fs, gcsfs, http) are read via concurrent range requests on the event loop of their filesystem (IndexedTarStore.read_async / is_remote). On event loops of applications, blocking work (local reads, decompression, zarr-python's codec pipeline) runs on worker threads, on the event loops of the calling threads it runs inline - zarr-python 2: reads run on worker threads Metadata (attributes, names, timestamps) is held in memory and only provided synchronously.
Collaborator
Author
|
/ok to test 772164c |
Collaborator
Author
|
Continued as a stack of pull requests, one per commit of this PR (same commits, unchanged):
The review history of this PR stays here for reference. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Supports both zarr-python 2 and zarr-python 3 in the same ncore version, and adds optional zarr format v3 data.
SequenceComponentGroupsWriter(..., zarr_format=3)/--zarr-format 3.Commits
The PR is split into commits that build and pass all tests individually, and can be merged in order:
test: make tests independent of scipy / numpy versionsbuild: python 3.12 / 3.13 test environments (zarr-python 2 everywhere)refactor(data):IndexedTarStoreimplemented independently of zarr-python (raw record API, positional reads for local archives; records byte-identical to ncore 19.8)feat(data)!: typed node API (nodes.Group/nodes.Array) for component groups, used by all components; zarr-python version specific operations behind a backend interfaceperf(data): node cache for component readers (node_cache_size)feat(data)!: zarr-python 3 backend (zarr format v2 data)feat(data)!: zarr format v3 stores,--zarr-formatfor convertersperf(data): direct chunk reads with zarr-python 3feat(data): asynchronous component reader API (*_asyncaccessors)Design notes
zarr.jsonwith the consolidated metadata as CBOR + zlib recordzarr.json.cbor.zlib.NCORE_ZARR3_DIRECT_READS=0disables this.Read performance
Full test sequence (35 stores / 6037 nodes, 312 camera frames / 408 MB, 8 lidar frames / 2.5M points, 213 radar frames), median of 5:
Frame reads are faster than
mainwith both zarr-python versions (node cache, positional reads, direct chunk reads). Opening stores loads all metadata up front from a single read per store, independent of local or remote storage, so the open time is CPU: zarr-python 3 constructs objects for all nodes of a consolidated hierarchy when opening it (~40 us per node, 6037 nodes), zarr-python 2 constructs them on access.Consolidated vs unconsolidated opening (zarr-python 3, node cache on, all 320 camera / lidar frames):
open_consolidated=Trueopen_consolidated=FalseUnconsolidated opening defers metadata reads to the first access of each node (cached afterwards), which is faster for local data. On remote storage each of these reads is a separate request, so consolidated opening (the default, a single read per store) is preferable there.
Testing
All tests run against four environments (dataset converter tests remain python 3.11 only):
_3_8_3_11_3_12_3_13test-data-v4-trimmed0.2).Breaking changes
zarr>=2.12.0, excluding zarr-python 3 versions before 3.1.4). Consumers using zarr-python 2 APIs directly should pinzarr<3until migrated.self._group) arenodes.Groupinstances (the wrapped zarr-python group is accessible via.zarr).stores.consolidate_compressed_metadata/open_compressed_consolidatedare replaced bystores.consolidate_store/open_store.BaseDataConverterConfig.zarr_formatis required.