Writer: refuse a mistyped stream, and mark unreadable VDS frames

Two ways the written files could misdescribe themselves without anyone noticing.

The pixel format is stated twice and the two were never compared: each image
carries its own type as a CBOR tag, which is what the data files are written
with, while the master is typed from the start message. A stream whose header
contradicts its images produced data files of one type under a master declaring
another, and with NXmxVDS, HDF5 then converts silently on every read.
HDF5DataFile::CreateFile now checks the two agree and refuses the run otherwise -
the point where the values first meet, so it covers every path into the writer.

Two test fixtures were relying on exactly that inconsistency. The HDF5 writer
tests wrote uint16 buffers under a JUNGFRAU experiment, which converts to photon
counts by default and so declares int16; they never read the pixels back, so it
went unnoticed. The receiver-lite tests feed frames from
compression_benchmark.h5, which really are signed int16, through a DECTRIS
experiment, which declares unsigned by default - the same class of bug the
pixel_signed propagation fixed on the live path. Both now declare what they send.

Second: a virtual dataset whose source file is absent reads as the fill value,
and HDF5 defaults that to zero, so a data file that was not copied alongside the
master is indistinguishable from frames of genuine zero counts. Measured with
DIALS on a four-file set with one file removed: 25 frames of pure zeros, no error
and no warning. The image VDS is now filled with the error marker instead, which
sits outside underload_value..saturation_value, so a reader masks those frames.
Same measurement after the change: -32768 throughout, which DIALS excludes.

Only the images ask for a fill value; the per-image metadata datasets keep the
default.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

docs/CHANGELOG.md

@@ -13,6 +13,8 @@ This is an UNSTABLE release. It includes many experimental features, as well as
 * rugnux: the `.poni` file written by `--mode calibration` no longer negates `Rot3`, which exported a detector rotation about the beam with the wrong sign.
 * Tests: a tilted detector is now cross-checked against pyFAI and DIALS, in the unit tests and against a written file in CI.
 * rugnux: the detector geometry is also logged in XDS's convention (`ORGX`/`ORGY`, detector axis vectors, rotation axis), so it can be compared directly with an XDS refinement.
+* HDF5: a data file missing next to a VDS master now reads as the error-pixel marker instead of zero counts, so those frames are masked rather than silently integrated as blank.
+* The writer refuses a stream whose start message declares a different pixel format than its images carry, instead of writing a master that does not describe its own data.
 
 ### 1.0.0-rc.161
 This is an UNSTABLE release. It includes many experimental features, as well as many AI generated fixes. We recommend using rc.152 for production use.

tests/HDF5WritingTest.cpp

@@ -478,7 +478,7 @@ TEST_CASE("HDF5Writer", "[HDF5][Full]") {
         x.FillMessage(start_message);
 
         FileWriter file_set(start_message);
-        std::vector<uint16_t> image(x.GetPixelsNum());
+        std::vector<int16_t> image(x.GetPixelsNum());
 
         for (int i = 0; i < x.GetImageNum(); i++) {
             DataMessage message{};
@@ -527,7 +527,7 @@ TEST_CASE("HDF5Writer_Socket", "[HDF5][Full]") {
 
         FileWriter file_set(start_message);
         file_set.SetupFinalizedFileSocket("ipc://#1");
-        std::vector<uint16_t> image(x.GetPixelsNum());
+        std::vector<int16_t> image(x.GetPixelsNum());
 
         ZMQSocket s(ZMQSocketType::Sub);
         s.Connect("ipc://#1");
@@ -601,7 +601,7 @@ TEST_CASE("HDF5Writer_Spots", "[HDF5][Full]") {
         x.FillMessage(start_message);
 
         FileWriter file_set(start_message);
-        std::vector<uint16_t> image(x.GetPixelsNum());
+        std::vector<int16_t> image(x.GetPixelsNum());
 
         for (int i = 0; i < x.GetImageNum(); i++) {
             DataMessage message{};
@@ -640,7 +640,7 @@ TEST_CASE("HDF5Writer_Rad_Int_Profile", "[HDF5][Full]") {
         start_message.az_int_q_bin_count = mapping.GetQBinCount();
 
         FileWriter file_set(start_message);
-        std::vector<uint16_t> image(x.GetPixelsNum());
+        std::vector<int16_t> image(x.GetPixelsNum());
 
         for (int i = 0; i < x.GetImageNum(); i++) {
             DataMessage message{};

tests/JFJochReceiverLiteTest.cpp

@@ -11,6 +11,10 @@
 #include "../image_pusher/HDF5FilePusher.h"
 #include "../image_puller/TestImagePuller.h"
 
+// PixelSigned(true) below is not incidental: the frames fed in come from compression_benchmark.h5,
+// which stores signed int16, while a DECTRIS experiment declares unsigned by default. The receiver
+// forwards images byte-for-byte, so the start message it re-emits has to describe them - otherwise
+// the master file declares a type the data files do not have, which HDF5DataFile now refuses.
 TEST_CASE("JFJochReceiverLite", "[JFJochReceiver]") {
     Logger logger("JFJochReceiverLite");
 
@@ -21,7 +25,7 @@ TEST_CASE("JFJochReceiverLite", "[JFJochReceiver]") {
     DiffractionExperiment experiment(DetDECTRIS(2068, 2164, "Test", {}));
 
     experiment.ImagesPerTrigger(5).NumTriggers(1).UseInternalPacketGenerator(true).ImagesPerFile(2)
-            .FilePrefix("crystal_test_lite").JungfrauConvPhotonCnt(false).SetFileWriterFormat(
+            .FilePrefix("crystal_test_lite").JungfrauConvPhotonCnt(false).PixelSigned(true).SetFileWriterFormat(
                     FileWriterFormat::NXmxVDS).OverwriteExistingFiles(true)
             .DetectorDistance_mm(75).BeamY_pxl(1136).BeamX_pxl(1090).IncidentEnergy_keV(12.4)
             .SetUnitCell(UnitCell{.a = 36.9, .b = 78.95, .c = 78.95, .alpha =90, .beta = 90, .gamma = 90});
@@ -108,7 +112,7 @@ TEST_CASE("JFJochReceiverLite_Cancel", "[JFJochReceiver]") {
     DiffractionExperiment experiment(DetDECTRIS(2068, 2164, "Test", {}));
 
     experiment.ImagesPerTrigger(5).NumTriggers(1).UseInternalPacketGenerator(true).ImagesPerFile(2)
-            .FilePrefix("crystal_test_lite").JungfrauConvPhotonCnt(false).SetFileWriterFormat(
+            .FilePrefix("crystal_test_lite").JungfrauConvPhotonCnt(false).PixelSigned(true).SetFileWriterFormat(
                     FileWriterFormat::NXmxVDS).OverwriteExistingFiles(true)
             .DetectorDistance_mm(75).BeamY_pxl(1136).BeamX_pxl(1090).IncidentEnergy_keV(12.4)
             .SetUnitCell(UnitCell{.a = 36.9, .b = 78.95, .c = 78.95, .alpha =90, .beta = 90, .gamma = 90});

writer/HDF5DataFile.cpp

@@ -22,7 +22,9 @@
 HDF5DataFile::HDF5DataFile(const StartMessage &msg, uint64_t file_number, const std::string &filename) :
 filename(filename),
 file_number(file_number),
-write_images(msg.write_images.value_or(true)) {
+write_images(msg.write_images.value_or(true)),
+declared_bit_depth(msg.bit_depth_image),
+declared_pixel_signed(msg.pixel_signed) {
     if (msg.overwrite.has_value())
         overwrite = msg.overwrite.value();
 
@@ -135,6 +137,21 @@ void HDF5DataFile::CreateFile(const DataMessage& msg, std::shared_ptr<HDF5File>
     if (write_images) {
         HDF5Dcpl dcpl;
 
+        // The only point where the two independent statements of the pixel format meet: the images
+        // carry their own type (a CBOR tag per image, which is what the data files are written with)
+        // while the master is typed from the START message. Nothing else compares them, so a stream
+        // whose header contradicts its images produced data files of one type under a master
+        // declaring another - and with NXmxVDS, HDF5 then converts silently on every read.
+        if (msg.image.GetByteDepth() * 8 != declared_bit_depth
+            || msg.image.IsSigned() != declared_pixel_signed)
+            throw JFJochException(JFJochExceptionCategory::InputParameterInvalid,
+                                  "Image is " + std::to_string(msg.image.GetByteDepth() * 8)
+                                  + "-bit " + (msg.image.IsSigned() ? "signed" : "unsigned")
+                                  + ", but the start message declares "
+                                  + std::to_string(declared_bit_depth) + "-bit "
+                                  + (declared_pixel_signed ? "signed" : "unsigned")
+                                  + " - the master file would not describe the data it links to");
+
         HDF5DataType data_type(msg.image.GetMode());
 
         xpixel = msg.image.GetWidth();

writer/HDF5DataFile.h

@@ -25,6 +25,11 @@ class HDF5DataFile {
     const uint64_t file_number;
     const bool write_images;
 
+    // Pixel format the START message declared. The data files are typed from the images themselves
+    // and the master from the start message, so these two have to agree - see CreateFile.
+    const uint64_t declared_bit_depth;
+    const bool declared_pixel_signed;
+
     std::string tmp_filename;
 
     std::shared_ptr<HDF5File> data_file = nullptr;

writer/HDF5NXmx.cpp

@@ -119,7 +119,8 @@ void NXmx::LinkToData_VDS(const StartMessage &start, const EndMessage &end) {
         auto data_dataset = VDS(start,
                                 "/entry/data/data",
                                 {total_images, height, width},
-                                HDF5DataType(start.bit_depth_image / 8, start.pixel_signed));
+                                HDF5DataType(start.bit_depth_image / 8, start.pixel_signed),
+                                start.error_value);
         data_dataset->Attr("image_nr_low", (int32_t) 1)
                 .Attr("image_nr_high",(int32_t) total_images);
 
@@ -281,18 +282,44 @@ void NXmx::LinkToReflections_VDS(const StartMessage &start, const EndMessage &en
     }
 }
 
+namespace {
+    void SetFillValue(HDF5Dcpl &dcpl, const HDF5DataType &data_type,
+                      const std::optional<int64_t> &fill_value) {
+        if (!fill_value.has_value())
+            return;
+        const int64_t value = fill_value.value();
+        if (data_type.IsSigned()) {
+            switch (data_type.GetElemSize()) {
+                case 1: dcpl.SetFillValue8(static_cast<int8_t>(value)); break;
+                case 2: dcpl.SetFillValue16(static_cast<int16_t>(value)); break;
+                case 4: dcpl.SetFillValue32(static_cast<int32_t>(value)); break;
+                default: break;
+            }
+        } else {
+            switch (data_type.GetElemSize()) {
+                case 1: dcpl.SetFillValueU8(static_cast<uint8_t>(value)); break;
+                case 2: dcpl.SetFillValueU16(static_cast<uint16_t>(value)); break;
+                case 4: dcpl.SetFillValueU32(static_cast<uint32_t>(value)); break;
+                default: break;
+            }
+        }
+    }
+}
+
 std::unique_ptr<HDF5DataSet> NXmx::VDS(const StartMessage &start,
                                        const std::string &name,
                                        const std::vector<hsize_t> &dim,
-                                       const HDF5DataType &data_type) {
-    return VDS(start, name, name, dim, data_type);
+                                       const HDF5DataType &data_type,
+                                       const std::optional<int64_t> &fill_value) {
+    return VDS(start, name, name, dim, data_type, fill_value);
 }
 
 std::unique_ptr<HDF5DataSet> NXmx::VDS(const StartMessage &start,
                                        const std::string &name_src,
                                        const std::string &name_dest,
                                        const std::vector<hsize_t> &dim,
-                                       const HDF5DataType &data_type) {
+                                       const HDF5DataType &data_type,
+                                       const std::optional<int64_t> &fill_value) {
     if (dim.empty() || dim.size() > 3)
         throw JFJochException(JFJochExceptionCategory::InputParameterInvalid,
                               "Dimension must be in range 1-3");
@@ -312,6 +339,13 @@ std::unique_ptr<HDF5DataSet> NXmx::VDS(const StartMessage &start,
     if (dim.size() == 3)
         dcpl.SetChunking({1, dim[1], dim[2]});
 
+    // Where a virtual dataset has no source file to read from, HDF5 hands back the fill value, and
+    // that defaults to zero - so a data file that was not copied alongside the master reads as
+    // frames of zero counts, with no error and no warning. Fill with the error marker instead: it is
+    // outside underload_value..saturation_value, so a reader masks those frames rather than
+    // integrating them.
+    SetFillValue(dcpl, data_type, fill_value);
+
     for (hsize_t file_id = 0; file_id < file_count; file_id++) {
         hsize_t images_in_file = images_per_file;
         if (file_id == file_count - 1)

writer/HDF5NXmx.h

@@ -25,16 +25,20 @@ class NXmx {
     void LinkToData_ProcessingVDS(const StartMessage &start, const EndMessage &end);
     void LinkToReflections_VDS(const StartMessage &start, const EndMessage &end);
 
+    // fill_value, when given, is what the dataset reads as where no source file is available - see
+    // the note in HDF5NXmx.cpp. Left unset for everything but the images.
     std::unique_ptr<HDF5DataSet> VDS(const StartMessage &start,
                                      const std::string& name,
                                      const std::vector<hsize_t> &dim,
-                                     const HDF5DataType &data_type);
+                                     const HDF5DataType &data_type,
+                                     const std::optional<int64_t> &fill_value = {});
 
     std::unique_ptr<HDF5DataSet> VDS(const StartMessage &start,
                                      const std::string& name_src,
                                      const std::string& name_dest,
                                      const std::vector<hsize_t> &dim,
-                                     const HDF5DataType &data_type);
+                                     const HDF5DataType &data_type,
+                                     const std::optional<int64_t> &fill_value = {});
 
     void Detector(const StartMessage &start);
     void Detector(const StartMessage &start, const EndMessage &end);

writer/HDF5Objects.cpp

@@ -331,6 +331,10 @@ void HDF5Dcpl::SetFillValueU16(uint16_t val) {
     H5Pset_fill_value(id, H5T_NATIVE_UINT16, &val);
 }
 
+void HDF5Dcpl::SetFillValueU8(uint8_t val) {
+    H5Pset_fill_value(id, H5T_NATIVE_UINT8, &val);
+}
+
 void HDF5Dcpl::SetVirtual(const std::string &path, const std::string &dataset, const HDF5DataSpace &src_dataspace, const HDF5DataSpace &dest_dataspace) {
     layout = HDF5DataSetLayout::VIRTUAL;
     std::string filename = ExtractFilename(path);

writer/HDF5Objects.h

@@ -106,6 +106,7 @@ public:
     void SetFillValue8(int8_t val);
     void SetFillValueU32(uint32_t val);
     void SetFillValueU16(uint16_t val);
+    void SetFillValueU8(uint8_t val);
     void SetChunking(const std::vector<hsize_t> & dims);
     std::vector<hsize_t> GetChunking();
     CompressionAlgorithm GetCompression();