From 86b55e79d88bcba4e426082a2adfcc0d0067c64a Mon Sep 17 00:00:00 2001 From: Kai Zhao Date: Wed, 16 Sep 2026 20:26:45 -0700 Subject: [PATCH] Stop ALGO_BIOMD writing indeterminate bytes, and charge its load() SZBioMDDecomposition::save() writes firstFillFrame_ and fillValue_ unconditionally, but only compress_2d and compress_3d assign them. A 1D compression therefore put 12 bytes of whatever the object's memory last held into the stream: the same input compressed five times produced five different files. PR #145 gave the same two members an initialiser in SZBioMDXtcDecomposition, which was written from a copy of this file, and left the original. load() then took back everything it had charged. c_pos is the cursor before the reads, so c_pos - c is negative and the last line added the 37 bytes the bounded read() calls had just subtracted. Every later parse -- the encoder's load, the bin count, the decode -- ran on a budget that large again. The Xtc class has no such line. A sweep over the eleven algorithms at one, two and three dimensions puts the determinism defect in ALGO_BIOMD's 1D path alone; of the seventeen places that adjust remaining_length by hand, this is the only one that subtracts a difference taken in the wrong order. test_decomposition_save_load.cpp covers both halves of the contract for both decompositions at all three dimensionalities. Determinism is checked by constructing the decomposition over storage filled with two different patterns and comparing what save() writes, which does not depend on the allocator handing back dirty memory. Reverting either fix fails exactly the cases that fix addresses: the 1D determinism case, and the three accounting cases with "advanced 37 bytes but charged 0". Co-Authored-By: Claude Opus 5 --- .../decomposition/SZBioMDDecomposition.hpp | 7 +- .../modules/test_decomposition_save_load.cpp | 154 ++++++++++++++++++ 2 files changed, 156 insertions(+), 5 deletions(-) create mode 100644 tools/test/modules/test_decomposition_save_load.cpp diff --git a/include/SZ3/decomposition/SZBioMDDecomposition.hpp b/include/SZ3/decomposition/SZBioMDDecomposition.hpp index 0499dd7c0..ffb5c777f 100644 --- a/include/SZ3/decomposition/SZBioMDDecomposition.hpp +++ b/include/SZ3/decomposition/SZBioMDDecomposition.hpp @@ -50,13 +50,10 @@ class SZBioMDDecomposition : public concepts::DecompositionInterface } 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; } // void clear() { @@ -346,8 +343,8 @@ class SZBioMDDecomposition : public concepts::DecompositionInterface Quantizer quantizer; Config conf; int site = 0; - size_t firstFillFrame_; - T fillValue_; + size_t firstFillFrame_ = 0; + T fillValue_ = 0; }; template diff --git a/tools/test/modules/test_decomposition_save_load.cpp b/tools/test/modules/test_decomposition_save_load.cpp new file mode 100644 index 000000000..ad54031c5 --- /dev/null +++ b/tools/test/modules/test_decomposition_save_load.cpp @@ -0,0 +1,154 @@ +// The two halves of the save/load contract, for the decompositions the MD algorithms use. +// +// save() has to write the same bytes for the same input, or a compressed file cannot be +// checksummed and two runs of the same pipeline produce different output. +// +// A member that save() writes but some compress() path never assigns takes whatever the memory +// held before, so the check has to control what that was: the decomposition is constructed over +// a buffer filled with two different patterns, and the two headers compared. Compressing twice +// on the heap instead would only catch it when the allocator happens to hand back dirty memory. + +#include +#include +#include +#include +#include + +#include "SZ3/decomposition/SZBioMDDecomposition.hpp" +#include "SZ3/decomposition/SZBioMDXtcDecomposition.hpp" +#include "SZ3/quantizer/LinearQuantizer.hpp" +#include "SZ3/utils/Config.hpp" +#include "gtest/gtest.h" + +namespace { + +std::vector noise(size_t count, uint32_t seed = 11) { + std::mt19937 rng(seed); + std::uniform_real_distribution spread(-5.0f, 5.0f); + std::vector data(count); + for (auto &value : data) { + value = spread(rng); + } + return data; +} + +/// Compress over storage holding `poison`, and return what save() writes. +template +std::vector header_after_compress(const std::vector &dims, unsigned char poison, + MakeQuantizer make_quantizer) { + SZ3::Config conf; + conf.setDims(dims.begin(), dims.end()); + conf.errorBoundMode = SZ3::EB_ABS; + conf.absErrorBound = 1e-3; + + std::vector storage(sizeof(Decomposition) + alignof(Decomposition)); + std::memset(storage.data(), poison, storage.size()); + void *place = storage.data(); + size_t room = storage.size(); + place = std::align(alignof(Decomposition), sizeof(Decomposition), place, room); + + auto data = noise(conf.num); + auto *decomposition = new (place) Decomposition(conf, make_quantizer(conf)); + decomposition->compress(conf, data.data()); + + std::vector header(decomposition->size_est() + 4096); + SZ3::uchar *cursor = header.data(); + decomposition->save(cursor); + header.resize(static_cast(cursor - header.data())); + decomposition->~Decomposition(); + return header; +} + +template +void expect_header_depends_only_on_input(const std::vector &dims, MakeQuantizer make_quantizer) { + const auto over_zeros = header_after_compress(dims, 0x00, make_quantizer); + const auto over_ones = header_after_compress(dims, 0xcd, make_quantizer); + ASSERT_EQ(over_zeros.size(), over_ones.size()); + EXPECT_EQ(over_zeros, over_ones) << "save() wrote bytes that came from the memory it was built over"; +} + +/// load() must charge remaining_length for exactly what it read, which is what keeps every +/// later parse inside the buffer. +template +void expect_load_charges_what_it_reads(const std::vector &dims, MakeQuantizer make_quantizer) { + SZ3::Config conf; + conf.setDims(dims.begin(), dims.end()); + conf.errorBoundMode = SZ3::EB_ABS; + conf.absErrorBound = 1e-3; + + Decomposition writer(conf, make_quantizer(conf)); + auto data = noise(conf.num); + writer.compress(conf, data.data()); + + std::vector stream(writer.size_est() + 4096); + SZ3::uchar *write_cursor = stream.data(); + writer.save(write_cursor); + const size_t written = static_cast(write_cursor - stream.data()); + + Decomposition reader(conf, make_quantizer(conf)); + const SZ3::uchar *read_cursor = stream.data(); + size_t remaining = stream.size(); + reader.load(read_cursor, remaining); + + const size_t advanced = static_cast(read_cursor - stream.data()); + EXPECT_EQ(advanced, written) << "load() did not consume what save() wrote"; + EXPECT_EQ(stream.size() - remaining, advanced) + << "load() advanced " << advanced << " bytes but charged " << stream.size() - remaining; +} + +auto linear_quantizer = [](const SZ3::Config &conf) { + return SZ3::LinearQuantizer(conf.absErrorBound, conf.quantbinCnt / 2); +}; +auto xtc_quantizer = [](const SZ3::Config &conf) { + return SZ3::LinearQuantizer(conf.absErrorBound, SZ3::XTC_radius, false); +}; + +} // namespace + +TEST(SZ3_DecompositionSaveLoad, BioMDOneDimension) { + expect_header_depends_only_on_input>>( + {4096}, linear_quantizer); +} + +TEST(SZ3_DecompositionSaveLoad, BioMDTwoDimensions) { + expect_header_depends_only_on_input>>( + {64, 64}, linear_quantizer); +} + +TEST(SZ3_DecompositionSaveLoad, BioMDThreeDimensions) { + expect_header_depends_only_on_input>>( + {5, 777, 3}, linear_quantizer); +} + +TEST(SZ3_DecompositionSaveLoad, BioMDXtcOneDimension) { + expect_header_depends_only_on_input>>( + {4096}, xtc_quantizer); +} + +TEST(SZ3_DecompositionSaveLoad, BioMDXtcTwoDimensions) { + expect_header_depends_only_on_input>>( + {64, 64}, xtc_quantizer); +} + +TEST(SZ3_DecompositionSaveLoad, BioMDXtcThreeDimensions) { + expect_header_depends_only_on_input>>( + {5, 777, 3}, xtc_quantizer); +} + +TEST(SZ3_DecompositionSaveLoad, BioMDChargesWhatItReads) { + expect_load_charges_what_it_reads>>( + {4096}, linear_quantizer); + expect_load_charges_what_it_reads>>( + {64, 64}, linear_quantizer); + expect_load_charges_what_it_reads>>( + {5, 777, 3}, linear_quantizer); +} + +TEST(SZ3_DecompositionSaveLoad, BioMDXtcChargesWhatItReads) { + expect_load_charges_what_it_reads>>( + {4096}, xtc_quantizer); + expect_load_charges_what_it_reads>>( + {64, 64}, xtc_quantizer); + expect_load_charges_what_it_reads>>( + {5, 777, 3}, xtc_quantizer); +}