diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 4f2b51e6e..17b5c07d8 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -2,6 +2,7 @@ ## 1.0.0 ### 1.0.0-rc.166 * `rugnux` reads a PILATUS miniCBF rotation sweep natively, with no conversion: naming any frame - or the directory holding it - processes the whole sweep that frame's template belongs to. `--mode scale` still needs a `_process.h5`. +* A master whose companion `_meta.h5` was not kept still opens: the fields an Eiger master links into that file are treated as absent rather than as a read error, and a run that finds no saturation value anywhere says so and carries on instead of refusing the dataset. * `jfjoch_viewer` opens PILATUS miniCBF sweeps - naming any frame opens the whole sweep - and can run a processing job on one. * A miniCBF sweep takes its rotation axis from the goniometer angles the header states, instead of assuming the axis every such file was previously assumed to have. * A miniCBF sweep takes the mounting from the imgCIF axis table its header carries - which laboratory direction the image's columns and rows run along, and which the spindle turns about - and, where there is no table, from a `+SLOW` on the `Oscillation_axis` line, which says the spindle runs along the image's slow direction. diff --git a/reader/HDF5MetadataSource.cpp b/reader/HDF5MetadataSource.cpp index 20bef0df3..65a0bbeb6 100644 --- a/reader/HDF5MetadataSource.cpp +++ b/reader/HDF5MetadataSource.cpp @@ -986,10 +986,26 @@ HDF5MetadataSource::OpenResult HDF5MetadataSource::Open(const std::string &filen ReadLength_m(*master_file, "/entry/instrument/detector/sensor_thickness") * 1e6); if (master_file->Exists("/entry/instrument/detector/sensor_material")) detector.SensorMaterial(master_file->GetString("/entry/instrument/detector/sensor_material")); - detector.SaturationLimit(SaturationLimitFromValue( - ReadIntWithLegacyFallback(*master_file, - "/entry/instrument/detector/saturation_value", - "/entry/instrument/detector/detectorSpecific/countrate_correction_count_cutoff"))); + // Optional, because a file that states no saturation value anywhere is a real and common + // thing: an Eiger master links saturation_value into a companion _meta.h5, and a deposited + // dataset frequently does not include that file, leaving neither the NXmx name nor the + // DECTRIS one readable. Left unset, DiffractionExperiment::GetSaturationLimit() falls back + // to the container's own overflow, which is the safe direction - it can only fail to call a + // pixel saturated, where too LOW a value drops the whole reflection and silently removes the + // strongest data (see BitDepthImage below). Refusing the file outright is the one option that + // helps nobody. + if (master_file->Exists("/entry/instrument/detector/saturation_value") + || master_file->Exists("/entry/instrument/detector/detectorSpecific/countrate_correction_count_cutoff")) + detector.SaturationLimit(SaturationLimitFromValue( + ReadIntWithLegacyFallback(*master_file, + "/entry/instrument/detector/saturation_value", + "/entry/instrument/detector/detectorSpecific/countrate_correction_count_cutoff"))); + else + Logger("HDF5Reader").Warning("The file states no saturation value - neither NXmx saturation_value nor " + "the DECTRIS countrate_correction_count_cutoff is readable, which is what " + "an Eiger master looks like when its companion _meta.h5 was not kept. No " + "pixel will be called saturated; if this detector overloads, its strongest " + "reflections will be integrated as if they were valid."); // The reader hands every image out as signed int32 whatever the file stored (see PixelSigned // below), so that is the container depth the rest of the code has to see. DetectorSetup defaults // DECTRIS to 16 bits and GetByteDepthImage() prefers the detector's value over the image diff --git a/tests/HDF5WritingTest.cpp b/tests/HDF5WritingTest.cpp index 535e13e55..99a0f9b1a 100644 --- a/tests/HDF5WritingTest.cpp +++ b/tests/HDF5WritingTest.cpp @@ -1451,3 +1451,34 @@ TEST_CASE("HDF5FilePusher_finalize_failure_recovers", "[HDF5FilePusher][Repro]") std::filesystem::remove(e.path()); REQUIRE(H5Fget_obj_count(H5F_OBJ_ALL, H5F_OBJ_ALL) == 0); } + +// A link that is written in the file and points at something not there is NOT an existing dataset. +// Every DECTRIS Eiger master links saturation_value, pixel_mask, bit_depth_readout and +// serial_number into a companion _meta.h5, and that file is routinely not kept when a +// dataset is archived or deposited. Asking only whether the LINK exists then answers yes and the +// read that follows throws, which turns every optional-field guard in the reader into a hard +// failure - measured on a deposited Eiger 16M set that could not be opened at all. +TEST_CASE("HDF5Objects_dangling_external_link_does_not_exist", "[HDF5][Unit]") { + const std::string fname = "test_dangling_link.h5"; + remove(fname.c_str()); + { + HDF5File file(fname); + HDF5Group group(file, "/entry"); + group.SaveScalar("present", static_cast(7)); + // ...and a link into a file that does not exist, exactly as an orphaned Eiger master has. + REQUIRE(H5Lcreate_external("no_such_meta.h5", "/_dectris/whatever", + group.GetID(), "absent", H5P_DEFAULT, H5P_DEFAULT) >= 0); + } + { + HDF5ReadOnlyFile file(fname); + CHECK(file.Exists("/entry/present")); + CHECK(file.GetInt("/entry/present") == 7); + // The link is there, the object is not. + CHECK(H5Lexists(file.GetID(), "/entry/absent", H5P_DEFAULT) > 0); + CHECK_FALSE(file.Exists("/entry/absent")); + // ...so an optional read of it falls back instead of throwing. + CHECK(file.GetString("/entry/absent", "fallback") == "fallback"); + } + remove(fname.c_str()); + REQUIRE(H5Fget_obj_count(H5F_OBJ_ALL, H5F_OBJ_ALL) == 0); +} diff --git a/writer/HDF5Objects.cpp b/writer/HDF5Objects.cpp index 9f951476f..5f3ef778f 100644 --- a/writer/HDF5Objects.cpp +++ b/writer/HDF5Objects.cpp @@ -1072,7 +1072,16 @@ std::string HDF5Object::GetString(const std::string &name, const std::string &de bool HDF5Object::Exists(const std::string &name) const { H5E_BEGIN_TRY { - return H5Lexists(GetID(), name.c_str(), H5P_DEFAULT) > 0; + // The LINK has to be there and the object it names has to be reachable. Those are two + // different questions for an external link, and every DECTRIS Eiger master asks the second + // one: it links saturation_value, pixel_mask, bit_depth_readout and serial_number into a + // companion _meta.h5 that is routinely not kept when a dataset is archived or + // deposited. H5Lexists then says yes - the link is written in the master - and the read that + // follows throws, which turns every optional-field guard in the reader into a hard failure + // and refuses the whole file over a value it was prepared to do without. + if (H5Lexists(GetID(), name.c_str(), H5P_DEFAULT) <= 0) + return false; + return H5Oexists_by_name(GetID(), name.c_str(), H5P_DEFAULT) > 0; } H5E_END_TRY; return false; }