Conversation
* Drop 31 no-op ALWAYS_INLINE keywords Benchmarking on Anvil (EPYC 7763; GCC 10.2/11.2/14.2, AOCC clang 12) and locally (GCC 16.2, AppleClang) shows the attribute changes nothing at 31 of the 49 sites: the compilers already inline these, and removing the keyword either yields a byte-identical binary or measures zero. What the data says to keep, and why this is not a bulk removal: LinearQuantizer (5) + BlockwiseIterator (9) carry the entire effect. Downgrading just those two groups costs ~7.5%; downgrading everything else together costs ~0.2%. No single group crosses GCC's inlining threshold on its own, so per-group numbers hide this -- it only shows up when the two are isolated. They keep the attribute. Config.hpp to_lower keeps its inline: it is the only namespace-scope non-template among the sites, and without it the library stops linking. interp_akima / interp_pchip keep theirs: both are multi-line with branches, both currently have no callers, so the byte-identical result for them is vacuous and says nothing about future use. QuantOptimization.hpp lorenzo_predict_3d keeps its attribute; the function is dead code today but is expected to come back. Deleted rather than downgraded to plain inline: 27 of the 31 are in-class definitions (implicitly inline) and 4 are namespace-scope templates (already weak/COMDAT), so no keyword is needed to preserve linkage. Included here are the two on ComposedPredictor::predict/estimate_error, which is where GCC 16 actually hard-errors: the class is itself a PredictorInterface and may nest, so speculative devirtualization recurses into itself until the inline budget is exhausted -- and a failed always_inline is an error, not a warning. That is what broke the MinGW CI job on the fz branch. Verified: builds clean under GCC 16.2 and AppleClang; ALGO_LORENZO_REG, ALGO_INTERP_LORENZO, ALGO_INTERP and ALGO_NOPRED all round-trip within the error bound on the in-tree smoke file and on miranda 384x384x256. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Fix SDRBench dataset URLs: Globus endpoint moved All eight integration-test datasets return 404. SDRBench moved to a new Globus endpoint, changing both the host and the path prefix: g-8d6b0.fd635.8443.data.globus.org/ds131.2/Data-Reduction-Repo/raw-data/ g-d0cd3f.fd635.8443.data.globus.org/raw-data/ exaalt-copper and exaalt-helium were also renamed upstream, from SDRBENCH-exaalt-* to SDRBENCH-EXAALT-*, so the host swap alone still left those two at 404. New URLs taken from https://sdrbench.github.io/datasets.html and confirmed reachable (HTTP 206 on a range request) for all eight. The integration test matrix covers exactly these datasets, so CI on this PR is the real check that each archive still unpacks to the filenames datasets.json expects. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
MGARD/SPERR fused paths kept as *FusedDecomposition and still drive ALGO_MGARD / ALGO_SPERR; the composable compositions take the default names. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR expands SZ3’s modular “fz” toolkit by adding several new CPU-side modules (quantizers, decompositions, preprocessors, and utilities), plus substantial new unit tests, and it fixes a set of pre-existing correctness issues in encoders/quantizers/decompositions without changing interface base classes or Config.
Changes:
- Add new composable modules (e.g.,
MultiLevelDecomposition/MGARD alias,PaSTRIDecomposition, new quantizers includingLevelQuantizer,LogDomainQuantizer, etc.) and supporting utilities (MultiLevelErrorBound,MultiLevelQuantization). - Add/expand unit tests for the new quantizers/decompositions and for serialization/UID correctness.
- Fix multiple pre-existing bugs across encoders/quantizers/decompositions and adjust wiring/docs/CI accordingly.
Reviewed changes
Copilot reviewed 51 out of 52 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/test/modules/test_quantizer.cpp | Expands quantizer coverage; adds UID uniqueness test; updates BitTruncation bins to uint64_t. |
| tools/test/modules/test_quantizer_log.cpp | New tests for LogDomainQuantizer (relative-bound behavior, serialization, edge cases). |
| tools/test/modules/test_quantizer_level.cpp | New, thorough tests for merged LevelQuantizer curves and bound guarantees. |
| tools/test/modules/test_pastri.cpp | New unit tests for PaSTRIDecomposition including error-bound and save/load roundtrips. |
| tools/test/modules/test_decomposition.cpp | Updates decomposition tests to target fused MGARD/SPERR implementations. |
| include/SZ3/utils/MultiLevelQuantization.hpp | Adds helpers to walk multi-resolution coefficient slabs and drive per-level quantizers. |
| include/SZ3/utils/MultiLevelErrorBound.hpp | Adds geometric per-level absolute error-bound scheduling utility. |
| include/SZ3/utils/KmeansUtil.hpp | Fixes UB sampling bug (reserve/[] and inclusive bound), but still needs a size_t distribution fix. |
| include/SZ3/utils/BlockwiseIterator.hpp | Adds block_data::values() helper for unpadded view of block data (currently rebuilds each call). |
| include/SZ3/quantizer/Quantizer.hpp | Adds quantizer_bin_t helper to deduce quantizer output type for decompositions. |
| include/SZ3/quantizer/QuadraticLevelQuantizer.hpp | Removes old quadratic-only LUT quantizer (superseded by LevelQuantizer). |
| include/SZ3/quantizer/OutlierQuantizer.hpp | Adds sparse outlier-correction quantizer; get_out_range() likely needs to signal “no range”. |
| include/SZ3/quantizer/LogDomainQuantizer.hpp | Adds log-domain quantizer guaranteeing pointwise relative error bounds with verbatim fallback. |
| include/SZ3/quantizer/LinearQuantizer.hpp | Adds bounds check on recover_unpred() to prevent buffer overrun during decode. |
| include/SZ3/quantizer/LevelQuantizer.hpp | Adds merged non-uniform LUT quantizer with quadratic/log curves and unconditional bound via unpredictable path. |
| include/SZ3/quantizer/GranularBitRoundQuantizer.hpp | Adds “bit grooming” style quantizer that rounds mantissa bits to preserve significant digits. |
| include/SZ3/quantizer/ClusterQuantizer.hpp | Adds cluster/codebook quantizer and a helper to derive lattice levels via k-means. |
| include/SZ3/quantizer/BitTruncationQuantizer.hpp | Switches bin type to uint64_t to avoid negative bins and align with (0,0) “no range” signaling. |
| include/SZ3/preprocessor/SPERRTransform.hpp | Adds standalone SPERR conditioning + wavelet preprocessor (composable transform half). |
| include/SZ3/preprocessor/PreFilter.hpp | Fixes pointer iteration by using dims to compute element count. |
| include/SZ3/preprocessor/MGARDTransform.hpp | Adds standalone MGARD multigrid transform preprocessor with level geometry utilities. |
| include/SZ3/encoder/ZFPEncoder.hpp | Fixes includes and loop types for warnings/correctness. |
| include/SZ3/encoder/RunlengthEncoder.hpp | Adds size_est() based on bin count to prevent buffer overruns. |
| include/SZ3/encoder/HuffmanEncoder.hpp | Adds guard against excessively wide bin ranges that would overflow/over-allocate state tables. |
| include/SZ3/encoder/BitshuffleEncoder.hpp | Fixes uninitialized output buffer (OR-only loops) and narrows bit ops to correct unsigned types. |
| include/SZ3/encoder/ArithmeticEncoder.hpp | Fixes sign-extension bug by shifting an unsigned value. |
| include/SZ3/decomposition/ZFPDecomposition.hpp | Fixes loop/index types and std::min typing to remove warnings/bugs. |
| include/SZ3/decomposition/TimeSeriesDecomposition.hpp | Fixes timestep-0 reconstruction usage for prediction to preserve error-bound correctness. |
| include/SZ3/decomposition/SVDDecomposition.hpp | Fixes warnings and simplifies shuffle permutation handling. |
| include/SZ3/decomposition/SPERRFusedDecomposition.hpp | Adds renamed fused SPERR decomposition (transform+quant+SPECK stream in state). |
| include/SZ3/decomposition/PaSTRIDecomposition.hpp | Adds PaSTRI ERI decomposition with serialization and unconditional bound via verbatim fallback. |
| include/SZ3/decomposition/NoPredictionDecomposition.hpp | Generalizes bin type using quantizer_bin_t instead of hard-coding int. |
| include/SZ3/decomposition/MultiLevelDecomposition.hpp | Adds composable multilevel transform+schedule+per-level-quantizer decomposition (MGARD alias). |
| include/SZ3/decomposition/MGARDFusedDecomposition.hpp | Renames and clarifies the fused MGARD decomposition implementation. |
| include/SZ3/decomposition/BlockwiseDecomposition.hpp | Generalizes bin type using quantizer_bin_t instead of hard-coding int. |
| include/SZ3/compressor/ZFPCompressor.hpp | Fixes loop types for correctness/warnings. |
| include/SZ3/compressor/SZGenericCompressor.hpp | Enforces encoder stateNum fits int; instructs decompositions to return (0,0) if no usable range. |
| include/SZ3/api/sz_dev.hpp | Exposes new modules via sz_dev.hpp and updates includes/formatting. |
| include/SZ3/api/impl/SZAlgoSPERR.hpp | Switches algorithm wiring to SPERRFusedDecomposition. |
| include/SZ3/api/impl/SZAlgoMGARD.hpp | Switches algorithm wiring to MGARDFusedDecomposition and updates doc comment. |
| docs/claude-skills/fz-overview/SKILL.md | Updates module listings/names to match new decompositions/quantizers. |
| docs/claude-skills/fz-compose-pipeline/SKILL.md | Updates example wiring names for MGARD/SPERR fused variants. |
| docs/claude-skills/fz-add-module/SKILL.md | Updates naming examples and UID list to reflect new/renamed quantizers/decompositions. |
| CLAUDE.md | Updates module lists and MGARD pipeline description to match current implementation. |
| .gitignore | Stops ignoring the entire test directory by changing test to /test. |
| .github/workflows/integration_test.yml | Runs integration workflow on PRs targeting fz branch as well as master. |
Suppressed comments (1)
include/SZ3/utils/KmeansUtil.hpp:314
get_cluster()buildsstd::uniform_int_distribution<>(i.e.,int) and castsnum(asize_t) down toint. Fornum > INT_MAXthis truncates/overflows and can yield out-of-bounds indices; fornum == 0it also produces an invalid upper bound. Use asize_tdistribution over[0, num-1].
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+225
to
+227
| std::pair<To, To> get_out_range() const override { | ||
| return std::make_pair(std::numeric_limits<To>::lowest(), std::numeric_limits<To>::max()); | ||
| } |
Comment on lines
+239
to
+246
| const T *values() { | ||
| if (padding == 0 || internal_buffer.empty()) { | ||
| return data_padding; | ||
| } | ||
| unpadded_buffer.resize(num); | ||
| copy_data_with_padding(unpadded_buffer.data(), ds, data_padding, ds_padding, dims); | ||
| return unpadded_buffer.data(); | ||
| } |
Comment on lines
+130
to
+135
| quantizers_.clear(); | ||
| quantizers_.reserve(level_ebs.size()); | ||
| for (double level_eb : level_ebs) { | ||
| quantizers_.push_back(make_quantizer_(level_eb)); | ||
| } | ||
|
|
libstdc++ does not provide std::make_shared transitively; the new test translation units changed the include order enough to expose it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Group-level contract checks (ModuleContract.hpp) and a header self-containment check, both runnable and wired into CI; docs/MODULE_ACCEPTANCE.md records what a module must satisfy to enter the library. Each check comes from a defect class that has shipped here. Fixes found by the two checks: 9 headers were not self-contained, and MemoryUtil::read guarded remaining_length with assert, which Release compiles out, so a truncated stream read past the buffer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The check only ran under libc++ locally, which provides more transitively; 17 headers were missing <limits>, <cmath>, <cstddef>, def.hpp or a project header. CI now runs the check on the macOS job as well, so a clean run under one standard library no longer passes for both. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
RegressionPredictor::load debited remaining_length by the decoded bin count (coeff_size * sizeof(int)) while decode() advances the cursor by the much smaller encoded byte count, so the budget ran out early. Harmless while the guard in MemoryUtil::read was an assert compiled out of Release builds; now that it throws, the HDF5 filter path hit it on miranda velocity fields at eb=1e-4. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
sizeof(T) * num * 2 is below SZ_compress's own minimum for a small chunk, so an explicit chunk of a few hundred elements aborted with "The buffer for compressed data is not large enough". The integration test now covers an explicit small chunk alongside the full and auto-chunked cases. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Six decompositions did not override size_est(), so SZGenericCompressor sized its buffer from 0 while the quantizer's unpredictable list could be arbitrarily large; three are fixed here and the rest already had one. The contract also records MGARD's bound limitation: it quantizes coefficients per level and never checks the reconstruction, so the transform's own round-off passes through and the absolute bound holds only while max|x| * epsilon<T> << eb. Also restores the H5Z filter's buffer headroom on top of SZ_compress's minimum. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
compress() memcpy'd srcLen bytes into dst regardless of dstCap, so any payload larger than the caller's buffer was an unconditional heap overflow. Lossless_zstd already checks and throws; bypass now does the same. Caught by AddressSanitizer on Linux: ALGO_BIOMDXTC pairs its codec with Lossless_bypass, so nothing shrinks the payload, and the HDF5 filter's buffer was sized from SZ_compress's minimum, which assumes it does. glibc reported it as "munmap_chunk(): invalid pointer"; macOS's allocator did not abort, which is why it only showed up in CI. The filter's headroom is restored here too - it was lost restoring a backup during an earlier A/B. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
decode() advances the cursor but leaves remaining_length untouched, so the bound check was too loose for everything read after it. Same defect class as RegressionPredictor::load, in the opposite direction. MemoryUtil::read now compares by division so the size computation cannot overflow on its own. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contracts now cover the Lossless group as well; its destination-capacity check is what Lossless_bypass was missing. Adds tests for SZGenericCompressor's two guards on get_out_range, which had none. GranularBitRoundQuantizer's tests move out of test_quantizer_cluster.cpp into their own file. tools/bench/module_bench.cpp (BUILD_SZ3_BENCH=ON) measures a composition's compression ratio, achieved error and throughput on a real dataset and prints CSV. Module characteristics can only be measured, not authored, so this is the piece the composition metadata will need. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
modules.json was missing 8 modules; CLAUDE.md's quantizer list predated five of them and had no preprocessor list at all. Both now also point at the acceptance gate and the benchmark tool. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ModuleContract.hpp moves to include/SZ3/testing/ so it ships with the library: the gate is meant to apply to modules submitted from outside this tree, which cannot reach a header under tools/test/. modules.json gains a `composition` block on the 13 modules that constrain what they can be composed with -- an encoder that cannot derive its own bin range, a decomposition that only works with one encoder, bins that are not a countable domain, the regime an error bound holds in, a measured bad pairing. These are the facts a composition engine needs before any measured characteristic, and each one is already enforced by a test or by the code. SZ3_ModulesJson checks the file against the tree so the two cannot drift. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
MGARD quantized coefficients per level and never checked the reconstruction, so the synthesis round-off -- which scales with the field's magnitude -- passed through. For float at eb = 1e-2 a magnitude of 1e6 reached 1.17x eb and 1e7 reached 6.25x; every other decomposition checks the value it writes back and diverts to an unpredictable list. Both MGARD compositions now replay the decoder's inverse transform and record what still misses into an OutlierQuantizer, carried in save() the way LinearQuantizer carries its unpredictable list. Costs an inverse transform and a copy of the input on the compress side: miranda velocityx 301 MB, 500 -> 293 MB/s compressing, decompression and compression ratio unchanged (201.35 vs 201.36 at relative eb 1e-2). Changes the ALGO_MGARD stream only; the other seven algorithms are byte-identical. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The bound now holds unconditionally, but not for free everywhere: outliers cost nothing where the transform already met the bound, trade compression for the bound where it did not, and degenerate to near-lossless on a field that leaves the per-level quantizer saturated. The measured numbers are in the header. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every push added a full matrix without retiring the one in flight, so the long integration jobs of older runs were evicted mid-test instead of finishing -- hacc and exaalt-helium died at 13-17 and 25-30 minutes across three runs while the same jobs completed in 2h10m and 46m on a branch that was not being pushed to repeatedly. master and fz still keep every commit's build result; only feature branches and superseded pull-request runs are cancelled. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ings Bug fixes only. Nothing here changes the compressed format or an interface signature, and every algorithm's output is byte-identical to master's (verified over 31 dataset/algorithm/error-bound combinations, plus decompressing files master produced). From the open pull requests, reviewed line by line: #131 Config::load read one byte past the end of the config blob #132 bounds-check the whole decompression path (see the two skips below) #133 shift-by-64 UB for a single-symbol Huffman tree #134 bounds-check HuffmanEncoderV2 tree loading #135 bounds-check XtcBasedEncoder's magicInts index; plug a leaked buffer #137 remaining_length accounting after Huffman decode in both predictors #138 non-finite float to int64_t cast UB in LinearQuantizer #139 scratch buffers leaked when compression throws; OMP chunk capacity was missing room for the size header Lossless_zstd writes Two parts of #132 are deliberately not taken: - It moves the quant_inds count from after the encoder's tree to before it so the tree is immediately followed by its encoded stream. That is a compressed format change and it is not versioned, so files written by any released SZ3 fail to decode. The bound it was buying is recovered instead by letting the caller supply it: HuffmanEncoder::set_decode_bound(), called by SZGenericCompressor once it has consumed the count, and by both predictors. load() no longer guesses the bound from its own buffer -- the tree and the stream are not required to share a buffer, and test_encoder.cpp puts them in separate ones. - It reinterprets LosslessInterface::decompress's `dstLen` as an input capacity when the caller provides the buffer. The declared contract is that it is an output, and callers do pass uninitialised values, so this fails nondeterministically (it broke LosslessTest.LosslessBypass here). The self-allocating branch, where a non-zero value is opt-in, is kept. Also fixed while reviewing: - HuffmanEncoderV2 sized its dense tables from an unbounded `maxval` read straight from the stream; #134 bounds the node count but not this. - HuffmanEncoderV2's node count is an int holding a value read as 64-bit, so the new bound is checked with an explicit sign test rather than an implicit conversion. - test_lossless.cpp passed an uninitialised size to decompress(). From the fz branch, verified to reproduce here: Lossless_bypass::compress ignored its destination capacity, so any payload larger than the caller's buffer was an unconditional heap overflow; TimeSeriesDecomposition violated its error bound 1.94x on the null-reference-frame path; ArithmeticEncoder sign-extended a shift, corrupting about half of all streams; HuffmanEncoder's stateNum narrowing overflowed on a wide bin range; RunlengthEncoder and two decompositions had no size_est(), so the compressor sized its buffer from 0; KmeansUtil had reserve+operator[] UB and an off-by-one read; the HDF5 filter sized its buffer below SZ_compress's own minimum; 24 headers were not self-contained under libstdc++ or libc++; .gitignore's bare `test` pattern hid every test file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Taking the fz-branch fixes file by file overwrote them. The magicInts index and the packed-data size both come from the compressed stream and were used unchecked. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
num_elements * sizeof(T1) can overflow on its own, which is the computation the check is supposed to guard. Matches the scalar overload and PR #132. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Takes the eight reviewed bug-fix pull requests. Where the same defect was fixed on both branches, the master-fixes wording wins (its messages name the buffer and use out_of_range); where fz has the module the fix belongs to, fz wins. git merged both branches' identical additions twice in BlockwiseIterator (values()) and RunlengthEncoder (size_est), which does not compile; the duplicates are removed. PR #132's bound on the internal decompression buffer is dropped here, and needs dropping on master too: it bounds that buffer by SZ_compress_size_bound, which is the size of the *output* buffer. compress() sizes the internal one as max(1000, 2 * (decomposition.size_est() + encoder.size_est() + sizeof(Q) * bins)), so for a 64-bit bin type it is far larger and valid streams are rejected -- BitplaneEncoder, BitTruncationQuantizer and FixedPointQuantizer all hit it. master's in-tree modules all emit int bins, so its own tests cannot reach this. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
It bounds that buffer by SZ_compress_size_bound, which is the size of the *output* buffer. compress() sizes the internal one as max(1000, 2 * (decomposition.size_est() + encoder.size_est() + sizeof(Q) * bins)), so for a 64-bit bin type it is far larger and valid streams are rejected. Every module in this tree emits int bins, so the bound happens to hold and these tests cannot reach it -- the fz branch has three modules that do (BitplaneEncoder, BitTruncationQuantizer, FixedPointQuantizer) and all three failed. A corrupted declared size is still caught, after the allocation, by the zstd frame check and the size comparison this PR adds to Lossless_zstd::decompress. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
PR #132 rejects N outside [1, 4] and dimensions whose product differs from the element count. Both hold for a config read from the end of a compressed stream, and neither holds for the HDF5 filter's cd_values: those carry placeholder zeros that set_local fills in later, as cdvalueHelper.py says in as many words. The check killed the whole filter -- 80 of cesm-atm's 160 integration cases aborted with "invalid number of dimensions", across every algorithm. The single-argument overload, which is what the filter and the OpenMP path use, now says so explicitly and skips the content checks; the bounded overload used by SZ_decompress keeps them. Verified against the real cesm-atm field through the HDF5 filter: five algorithms pass, and removing the guard reproduces the abort. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The check PR #132 adds for N outside [1, 4] and for dimensions whose product differs from the element count holds for a config read from the end of a compressed stream, and not for the HDF5 filter's cd_values: those carry placeholder zeros that set_local fills in later, as cdvalueHelper.py says in as many words. It killed the whole filter -- 80 of cesm-atm's 160 and 96 of exaalt-copper's 192 integration cases aborted with "invalid number of dimensions", across every algorithm. Verified against the real cesm-atm field through the HDF5 filter. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A bare "-j" means unlimited parallel compilation. This tree now has 56 translation units against master's 9, and they pull in MGARD, SPERR and Eigen, so the peak at link time is enough to get a 15 GB runner terminated: hacc died three times at exit 143 immediately after "100% Linking CXX executable sz3ToHDF5", with no error of its own, 86 GB of disk still free and every other dataset passing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Python block-buffers stdout into the log pipe, so everything the driver prints -- including the captured output of every test it runs -- is lost when the job is terminated. The log then stops at the last write from a subprocess, which is the build, and says nothing about where the run actually died. hacc has failed four times with exit 143 and a log that ends at the same line; that line is where the visible output stops, not where the failure is, and two hypotheses I drew from it were wrong for exactly that reason. Runs the driver with -u, and flushes around dataset preparation so the phase is identifiable even without it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
capture_output=True prints the child's output only after it exits, so a case that never returns leaves no trace of how far it got. hacc has now failed five times; the log ends at the "command:" line for its first case -- a single 1.12 GB field written as one chunk -- and everything that case printed is still invisible. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The mode asked for 8-element chunks over the whole field. On a 1D field of 280 million elements that is 35 million chunks, and the HDF5 chunk index alone took the process to 15.3 GiB of a CI runner's 16 GiB -- hacc and exaalt-helium were killed there. Restricting the mode to a leading slice of at most 4096 chunks brings the peak to 9.6 GiB and leaves the mode's coverage intact: an unfixed filter still fails it on every algorithm at ordinary dataset sizes. Also stop tracking a stray __pycache__ entry. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The helper's name-to-enum map stopped at ALGO_BIOMDXTC, so ALGO_SVD, ALGO_ZFP, ALGO_SPERR and ALGO_MGARD could not be requested through the HDF5 filter at all. All four round-trip correctly through the filter, on the in-tree smoke file and on a 256x384x384 field, in every chunk mode. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The filter has to size its output buffer from SZ_compress_size_bound rather than from the raw chunk size, and nothing in the suite exercised that: a chunk small enough for the compressed block to exceed it never appeared. This adds a third chunk mode that asks for 8-element chunks, which the pre-fix filter rejects with "buffer not large enough" on every algorithm. The mode is restricted to a leading slice of at most 4096 chunks. Over a whole field it would mean tens of millions of chunks, and the HDF5 chunk index alone takes the process past a CI runner's memory. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ayzk
changed the title
Bug fixes only: no compressed-format change and no interface signature change. Covers the reviewed content of #131, #133, #134, #135, #137, #138 and #139, plus the findings ported from the fz branch. #132 is only partly covered -- see the PR body for the four exclusions and the measurement behind each. 31 dataset x algorithm x error-bound combinations are byte-identical to master, and every master-produced file still decompresses to the same bytes.
master now carries the consolidated bug fixes (#144). Three conflicts, all in commentary: fz-dev had a stale one-line comment above Config::load's fuller doc block (dropped), one condition differed only in line wrapping (took master's single line, which fits the 120-column limit), and fz-dev's note on the dropped decompression-buffer bound names the modules that actually emit 64-bit bins, so it is the accurate wording here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
New modules
PaSTRIDecomposition(pattern-based prediction, ERI),MultiLevelDecomposition(+MGARDDecompositionalias),SPERRDecomposition(composable)MGARDTransform,SPERRTransformClusterQuantizer,GranularBitRoundQuantizer(Bit Grooming),LevelQuantizer,LogDomainQuantizer,OutlierQuantizerMultiLevelErrorBound,MultiLevelQuantizationAll inherit their group's interface. No interface class definition or
Configwas changed.MGARD / SPERR naming
The fused implementations are performance-validated and keep driving
ALGO_MGARD/ALGO_SPERR, renamed toMGARDFusedDecomposition/SPERRFusedDecomposition. The composable compositions take the default names.Fused vs composable on miranda pressure (144 MB,
LinearQuantizerboth sides):Max absolute error identical in every case. SPERR composable is ~19% faster both ways; MGARD composable is ~6% slower to decompress.
Bug fixes
All pre-existing, found mostly by the new composition tests and by running the suite under AddressSanitizer, GCC/libstdc++, and a memory-capped Linux container.
Correctness
ArithmeticEncoder>> 20sign-extends, corrupting ~half of streamsBitshuffleEncodermalloc'd bufferLevelQuantizerBitTruncationQuantizerdoubleTimeSeriesDecompositionMGARDFusedDecompositionOutlierQuantizerpassuidcollisions (now covered by a uniqueness test)Memory safety
Lossless_bypass::compressRunlengthEncoderand 6 other modulessize_est()→ heap overrunHuffmanEncoderstateNumnarrowing overflowHuffmanEncoderV2maxvalallocation from a corrupted streamRegressionPredictor::load,ComposedPredictor::loadremaining_lengthaccountingMemoryUtil::readassert, compiled out of ReleaseKmeansUtilreserve+operator[]UB, off-by-one OOB readrecover_unpred()had no bounds checkBuild and test infrastructure
.gitignoretest->/test— it was hiding every test file in the treecdvalueHelper's name-to-enum map stopped atALGO_BIOMDXTC, soALGO_SVD,ALGO_ZFP,ALGO_SPERRandALGO_MGARDcould not be requested through the HDF5 filter at allintegration_test.ymlnow also runs on PRs tofz, and one run per branch/PR at a timeOther
NoPredictionDecomposition/BlockwiseDecompositionno longer hardcodeTo = int; deduced viaquantizer_bin_tget_out_range()returns(0, 0)where there is no usable bin range;SZGenericCompressorrejects a range that cannot fit inpreprocess_encode'sintblock_data::values()gives the block's values in unpadded layoutsz_dev.hppwas missing 8 modulesSZ3_ModuleContract(group contracts, installed asinclude/SZ3/testing/ModuleContract.hpp),SZ3_ModulesJson, andtools/test/check_headers.pytools/bench/module_bench(BUILD_SZ3_BENCH=ON) measures a composition's CR, error and throughput on a datasetVerification
BUILD_TESTING=ON: 0 errors, 0 warnings, 186/186 testsALGO_ZFP,ALGO_MGARDandALGO_SPERRalso on a 256x384x384 field15.30 GiB and OOM-killed before, 9.64 GiB and passing after
🤖 Generated with Claude Code