diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 3c3f1ad8..addfd3d0 100644 --- a/docs/CHANGELOG.md +++ b/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. diff --git a/tests/HDF5WritingTest.cpp b/tests/HDF5WritingTest.cpp index 657c4bca..2f31ea4f 100644 --- a/tests/HDF5WritingTest.cpp +++ b/tests/HDF5WritingTest.cpp @@ -478,7 +478,7 @@ TEST_CASE("HDF5Writer", "[HDF5][Full]") { x.FillMessage(start_message); FileWriter file_set(start_message); - std::vector image(x.GetPixelsNum()); + std::vector 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 image(x.GetPixelsNum()); + std::vector 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 image(x.GetPixelsNum()); + std::vector 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 image(x.GetPixelsNum()); + std::vector image(x.GetPixelsNum()); for (int i = 0; i < x.GetImageNum(); i++) { DataMessage message{}; diff --git a/tests/JFJochReceiverLiteTest.cpp b/tests/JFJochReceiverLiteTest.cpp index dabaa5e5..3d19fb50 100644 --- a/tests/JFJochReceiverLiteTest.cpp +++ b/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}); diff --git a/writer/HDF5DataFile.cpp b/writer/HDF5DataFile.cpp index 88fc38ed..79e03f70 100644 --- a/writer/HDF5DataFile.cpp +++ b/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 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(); diff --git a/writer/HDF5DataFile.h b/writer/HDF5DataFile.h index 0979f99f..db905f7a 100644 --- a/writer/HDF5DataFile.h +++ b/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 data_file = nullptr; diff --git a/writer/HDF5NXmx.cpp b/writer/HDF5NXmx.cpp index 8f0261d8..3c877a71 100644 --- a/writer/HDF5NXmx.cpp +++ b/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 &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(value)); break; + case 2: dcpl.SetFillValue16(static_cast(value)); break; + case 4: dcpl.SetFillValue32(static_cast(value)); break; + default: break; + } + } else { + switch (data_type.GetElemSize()) { + case 1: dcpl.SetFillValueU8(static_cast(value)); break; + case 2: dcpl.SetFillValueU16(static_cast(value)); break; + case 4: dcpl.SetFillValueU32(static_cast(value)); break; + default: break; + } + } + } +} + std::unique_ptr NXmx::VDS(const StartMessage &start, const std::string &name, const std::vector &dim, - const HDF5DataType &data_type) { - return VDS(start, name, name, dim, data_type); + const HDF5DataType &data_type, + const std::optional &fill_value) { + return VDS(start, name, name, dim, data_type, fill_value); } std::unique_ptr NXmx::VDS(const StartMessage &start, const std::string &name_src, const std::string &name_dest, const std::vector &dim, - const HDF5DataType &data_type) { + const HDF5DataType &data_type, + const std::optional &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 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) diff --git a/writer/HDF5NXmx.h b/writer/HDF5NXmx.h index f79d09d6..a669d973 100644 --- a/writer/HDF5NXmx.h +++ b/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 VDS(const StartMessage &start, const std::string& name, const std::vector &dim, - const HDF5DataType &data_type); + const HDF5DataType &data_type, + const std::optional &fill_value = {}); std::unique_ptr VDS(const StartMessage &start, const std::string& name_src, const std::string& name_dest, const std::vector &dim, - const HDF5DataType &data_type); + const HDF5DataType &data_type, + const std::optional &fill_value = {}); void Detector(const StartMessage &start); void Detector(const StartMessage &start, const EndMessage &end); diff --git a/writer/HDF5Objects.cpp b/writer/HDF5Objects.cpp index 66aa2fe2..ae5f5258 100644 --- a/writer/HDF5Objects.cpp +++ b/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); diff --git a/writer/HDF5Objects.h b/writer/HDF5Objects.h index ba085b45..02c32d1b 100644 --- a/writer/HDF5Objects.h +++ b/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 & dims); std::vector GetChunking(); CompressionAlgorithm GetCompression();