Stop ALGO_BIOMD writing indeterminate bytes, and charge its load()

[ayzk]

Two defects in SZBioMDDecomposition, the decomposition behind ALGO_BIOMD. Both matter for
GROMACS, which is about to depend on this algorithm through the HDF5 filter.

The same input does not compress to the same file

save() writes firstFillFrame_ and fillValue_ unconditionally, and only compress_2d and
compress_3d assign them. A 1D compression puts 12 bytes of whatever the object's memory last
held into the stream. Current master, one input, five runs:

308baf4d6e253fd3563f36df289ef503  d1.sz
36dac7922e24e69ca5883391f696a28a  d2.sz
6a9c39852ba515b86d80edc2f3473d4d  d3.sz
1d9db36f3c1a361927c6580651ff4b06  d4.sz
ef8e43ac160ebcaadc4722f1face67dc  d5.sz

A trajectory cannot be checksummed, a pipeline cannot be re-run reproducibly, and content-addressed
storage of the resulting files does not deduplicate.

PR #145 gave the same two members an initialiser in SZBioMDXtcDecomposition, whose first line is
Based on SZBioMDDecomposition.hpp. The copy was fixed and the original was not.

load() gives back the bytes it charged

const uchar *c_pos = c;
read(site, c, remaining_length);
read(firstFillFrame_, c, remaining_length);
read(fillValue_, c, remaining_length);
quantizer.load(c, remaining_length);
remaining_length -= c_pos - c;   // c_pos is the cursor BEFORE the reads

c_pos - c is negative, so the last line adds back exactly what the bounded read() calls
subtracted. Measured: the cursor advances 37 bytes and remaining_length is charged 0. Everything
parsed afterwards — the encoder's load, the bin count, decode — runs on a budget 37 bytes
larger than the buffer holds. This is the accounting #145 existed to fix, left in place in the
algorithm GROMACS uses. SZBioMDXtcDecomposition::load() has no such line.

Neither defect is present anywhere else

Determinism, four compressions of one input, every algorithm at each dimensionality:

	1D	2D	3D
ALGO_BIOMD	4 distinct outputs	deterministic	deterministic
the other ten	deterministic	deterministic	deterministic

Of the seventeen places in include/SZ3 that adjust remaining_length by hand, this is the only
one that subtracts a difference taken in the wrong order. The rest are -= consumed or
-= <size>, several guarded by a preceding if (consumed > remaining_length) throw.

The fix

     void load(const uchar *&c, size_t &remaining_length) override {
-        //            clear();
-        const uchar *c_pos = c;
         read(site, c, remaining_length);
         read(firstFillFrame_, c, remaining_length);
         read(fillValue_, c, remaining_length);
         quantizer.load(c, remaining_length);
-        remaining_length -= c_pos - c;
     }
@@
     int site = 0;
-    size_t firstFillFrame_;
-    T fillValue_;
+    size_t firstFillFrame_ = 0;
+    T fillValue_ = 0;

Tests

tools/test/modules/test_decomposition_save_load.cpp: both halves of the save/load contract, for
both MD decompositions, at one, two and three dimensions.

Determinism is checked by constructing the decomposition over storage filled with two different
patterns and comparing what save() writes. Compressing twice on the heap instead would only catch
this when the allocator happens to return dirty memory — it fires at 4096 elements and not at
12288 on the same build. The dimensionality matrix also records that SZBioMDXtcDecomposition was
broken at 1D and 2D before #145 and never at 3D, so a future determinism test written only against
3D data is visibly not covering anything.

Reverting either fix fails exactly the cases that fix addresses, and nothing else:

# without the member initialisers
save() wrote bytes that came from the memory it was built over
[  FAILED  ] BioMDOneDimension          [  PASSED  ] 5 tests

# with the sign-inverted line back
load() advanced 37 bytes but charged 0  (x3, one per dimensionality)
[  PASSED  ] 7 tests

Verification

Linux, gcc 13.3, Release with BUILD_TESTING=ON: 0 warnings, 15/15 ctest. The compressed format is
unchanged for 2D and 3D; 1D streams differ from master only in the 12 bytes that were indeterminate
there, and master's own 1D streams differ from each other.

🤖 Generated with Claude Code

[Copilot]

🟢 Approval recommended

The fixes are covered by regression tests; only a minor include nit remains.

Pull request overview

Fixes nondeterministic 1D ALGO_BIOMD serialization and incorrect load() byte accounting.

Changes:

• Initialize fill-state members deterministically.
• Remove the incorrect remaining_length adjustment.
• Add save/load regression tests across dimensions.

File summaries

File	Summary
tools/test/modules/test_decomposition_save_load.cpp	Adds deterministic serialization and byte-accounting coverage; minor nit: include <cstdint> directly (1 vote).
include/SZ3/decomposition/SZBioMDDecomposition.hpp	Initializes serialized state and corrects load() accounting.

Review details

Suppressed comments (1)

tools/test/modules/test_decomposition_save_load.cpp:14

• uint32_t is used by noise() but this new test does not include <cstdint>. It currently compiles only because another header happens to provide the declaration transitively; add the standard header directly so the test is portable across supported standard libraries.

#include <random>

• Files reviewed: 2/2 changed files
• Comments generated: 0
• Review effort level: Lite

---

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

[ayzk]

Folded into #147 — the fix and the tests that cover it belong in one change.