diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 51799229f..f4ce37c6c 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -9,6 +9,9 @@ * `rugnux` writes the unmerged MTZ `_unmerged.mtz` by default; `--no-export-unmerged` skips it. * Every rotation run that determines its own space group also writes `_P1.mtz`, the same observations merged in P1, so a wrong space group can be re-merged, re-solved or re-refined without processing the images again; `--no-p1-crosscheck` declines it, and a run given `-S` writes nothing because its space group's centring absences were never integrated. * The space-group search names the setting the data support, so a screw or a 2-fold on the a or c axis is reported as such instead of costing the crystal its space group. +* A space-group candidate whose predicted absences were never measurable no longer outranks one whose absences are genuinely dead - an unmeasured zone was worth about 690 nats per reflection, which inverted the ranking outright. +* A CBF with no detector header is refused instead of being opened with a pixel size of zero and every pixel called saturated, and a header number that does not parse is reported as a malformed header. +* 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. * The primitive cell of an R- or H-centred lattice is that lattice: the change of basis was applied without the transposition the convention needs, which left the volume right and the cell wrong, and reached the path that re-seats a lattice into a space group fixed by hand. * A centred monoclinic lattice that reduces to the mI form keeps its centring: the character naming that reduced form was missing from the table, so such a cell was reported as triclinic. * A lattice keeps the symmetry it has whatever orientation it was refined in: the cell reduction now judges a structurally-zero scalar product against the size of the cell rather than against a fixed tolerance eight decades smaller, so a body-centred tetragonal lattice is no longer read as C-centred monoclinic on most rotations of itself. diff --git a/reader/JFJochCBFReader.cpp b/reader/JFJochCBFReader.cpp index c60033e11..6cc40182f 100644 --- a/reader/JFJochCBFReader.cpp +++ b/reader/JFJochCBFReader.cpp @@ -12,6 +12,7 @@ #include #include "../common/JFJochException.h" +#include "../common/Logger.h" #include "../common/JFJochMath.h" namespace { @@ -171,7 +172,11 @@ bool JFJochCBFReader::CanRead(const std::string &path) { if (!HasCBFExtension(std::filesystem::path(path))) return false; try { - return minicbf::ReadHeader(path).byte_offset; + // A pixel size as well as the compression: a byte-offset CBF with no "# Pixel_size" line has + // no geometry, and XDS writes its correction files in exactly that shape. Claiming one here + // would open it with a pixel size of zero. + const auto h = minicbf::ReadHeader(path); + return h.byte_offset && h.pixel_x_m > 0.0 && h.pixel_y_m > 0.0; } catch (const JFJochException &) { return false; } @@ -187,6 +192,14 @@ void JFJochCBFReader::ReadFiles(const std::string &path) { if (!header0_.byte_offset) throw JFJochException(JFJochExceptionCategory::InputParameterInvalid, "Unsupported CBF compression (only x-CBF_BYTE_OFFSET)"); + // Beside the compression, because a pixel size is what makes the file an IMAGE: every resolution, + // every scattering vector and the beam centre in millimetres scale by it, and the default of 0 + // collapses all of them without a word. A byte-offset CBF with no "# Pixel_size" line is not a + // detector image from this family - XDS writes its correction files in exactly that shape. + if (!(header0_.pixel_x_m > 0.0) || !(header0_.pixel_y_m > 0.0)) + throw JFJochException(JFJochExceptionCategory::InputParameterInvalid, + files_[0] + " has no pixel size in its header (no '# Pixel_size' line); " + "it carries a CBF binary section but is not a detector image"); dataset_ = std::make_shared(); dataset_->experiment = default_experiment; @@ -195,7 +208,18 @@ void JFJochCBFReader::ReadFiles(const std::string &path) { detector.PixelSize_um(static_cast(std::lround(header0_.pixel_x_m * 1e6))); detector.SensorThickness_um(static_cast(std::lround(header0_.thickness_m * 1e6))); detector.SensorMaterial(header0_.material); - detector.SaturationLimit(SaturationLimitFromValue(header0_.count_cutoff)); + // Only when the header states one. Count_cutoff defaults to 0 and SaturationLimitFromValue(0) is + // 1, so an absent line marked EVERY pixel at or above one count as saturated - the integration + // accept gate then drops the whole reflection and the run comes out empty for a reason nothing + // reports. 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. + if (header0_.count_cutoff > 0) + detector.SaturationLimit(SaturationLimitFromValue(header0_.count_cutoff)); + else + Logger("CBFReader").Warning("{} states no Count_cutoff, so no pixel will be called " + "saturated; if this detector overloads, its strongest " + "reflections will be integrated as if they were valid.", + files_[0]); // Images are handed out as signed 32-bit whatever the file stored, so that is the depth the rest // of the code must see; the real overflow is the header's Count_cutoff, set above. detector.BitDepthImage(32); diff --git a/reader/MiniCBF.cpp b/reader/MiniCBF.cpp index 55750921a..4cc2f836c 100644 --- a/reader/MiniCBF.cpp +++ b/reader/MiniCBF.cpp @@ -28,14 +28,33 @@ std::optional Match(const std::string &text, const char *pattern) { return m[1].str(); } +// The captures are character classes, not number grammars: "[\d.eE+-]+" matches "." and "+-", and +// "(\d+)" matches a digit string too long for int64. std::stod and std::stoll answer both with a raw +// std:: exception, which would leave the format probe below - CanRead catches JFJochException only - +// and reach the caller as an unhandled throw from merely LOOKING at a file. A header field that does +// not parse is a malformed header, so say that. double Num(const std::string &text, const char *pattern, double fallback = 0.0) { const auto s = Match(text, pattern); - return s.has_value() ? std::stod(*s) : fallback; + if (!s.has_value()) + return fallback; + try { + return std::stod(*s); + } catch (const std::exception &) { + throw JFJochException(JFJochExceptionCategory::InputParameterInvalid, + "Malformed number in CBF header: '" + *s + "'"); + } } int64_t Int(const std::string &text, const char *pattern, int64_t fallback = 0) { const auto s = Match(text, pattern); - return s.has_value() ? std::stoll(*s) : fallback; + if (!s.has_value()) + return fallback; + try { + return std::stoll(*s); + } catch (const std::exception &) { + throw JFJochException(JFJochExceptionCategory::InputParameterInvalid, + "Malformed integer in CBF header: '" + *s + "'"); + } } // A goniometer angle, or nothing where the head has no such axis. Writers spell that -9999, and @@ -44,7 +63,13 @@ std::optional Angle(const std::string &text, const char *pattern) { const auto s = Match(text, pattern); if (!s.has_value()) return {}; - const double v = std::stod(*s); + double v; + try { + v = std::stod(*s); + } catch (const std::exception &) { + throw JFJochException(JFJochExceptionCategory::InputParameterInvalid, + "Malformed angle in CBF header: '" + *s + "'"); + } if (v < -9998.0) return {}; return v; @@ -349,6 +374,15 @@ Header ReadCommon(const std::string &path, std::vector &buf, size_t &bi if (h.nelem <= 0 || h.nx <= 0 || h.ny <= 0 || h.nelem != h.nx * h.ny) throw JFJochException(JFJochExceptionCategory::InputParameterInvalid, "Inconsistent image dimensions in " + path); + // Without a pixel size there is no geometry at all - every resolution, every scattering vector + // and the beam centre in millimetres all scale by it - and the default of 0 collapses all of + // them silently. A byte-offset CBF carrying no "# Pixel_size" line is not a detector image from + // this family at all; XDS writes correction files in exactly that shape. Refuse it here rather + // than let it through with a geometry of zero. + if (!(h.pixel_x_m > 0.0) || !(h.pixel_y_m > 0.0)) + throw JFJochException(JFJochExceptionCategory::InputParameterInvalid, + path + " has no pixel size in its header (no '# Pixel_size' line); " + "it carries a CBF binary section but is not a detector image"); binary_start = *start; return h; diff --git a/tests/JFJochReaderTest.cpp b/tests/JFJochReaderTest.cpp index 505105085..937c55537 100644 --- a/tests/JFJochReaderTest.cpp +++ b/tests/JFJochReaderTest.cpp @@ -4005,3 +4005,78 @@ TEST_CASE("JFJochCBFReader_AxisTableStatesTheMounting", "[HDF5][Full]") { RemoveMiniCBFSweep("cbfaxis_turn"); } } + +namespace { + // A byte-offset CBF whose header lines the caller chooses, so a header that is MISSING something + // can be built. WriteMiniCBFSweep above always writes a complete PILATUS head. + void WriteRawMiniCBF(const std::string &name, const std::string &head_lines, + int64_t nx, int64_t ny) { + std::ostringstream head; + head << "###CBF: VERSION 1.5\n_array_data.header_contents\n" << head_lines + << "_array_data.data\n--CIF-BINARY-FORMAT-SECTION--\n" + << "Content-Type: application/octet-stream;\n" + << " conversions=\"x-CBF_BYTE_OFFSET\"\n" + << "Content-Transfer-Encoding: BINARY\n" + << "X-Binary-Size: " << nx * ny << "\n" + << "X-Binary-Element-Type: \"signed 32-bit integer\"\n" + << "X-Binary-Number-of-Elements: " << nx * ny << "\n" + << "X-Binary-Size-Fastest-Dimension: " << nx << "\n" + << "X-Binary-Size-Second-Dimension: " << ny << "\n\n"; + std::ofstream f(name, std::ios::binary); + const std::string text = head.str(); + f.write(text.data(), static_cast(text.size())); + f.write(reinterpret_cast(minicbf::BINARY_SEPARATOR), + sizeof(minicbf::BINARY_SEPARATOR)); + const std::vector zeros(static_cast(nx * ny), 0); + f.write(zeros.data(), static_cast(zeros.size())); + } +} + +// Three ways a CBF that is not a detector image, or is one with a hole in its head, used to be +// opened anyway - each of them silently, which is the failure this project cares most about. +TEST_CASE("JFJochCBFReader_incomplete_header_is_refused_not_misread", "[HDF5][Full]") { + const int64_t nx = 24, ny = 16; + + SECTION("a byte-offset CBF with no PILATUS header is not ours to read") { + // XDS writes its correction files in exactly this shape: a real byte-offset binary section + // and not one '#' line. Claiming it opened it with a pixel size of zero, which collapses + // every resolution and scattering vector the run computes. + WriteRawMiniCBF("cbfbare_0001.cbf", "", nx, ny); + CHECK_FALSE(JFJochCBFReader::CanRead("cbfbare_0001.cbf")); + JFJochCBFReader reader; + CHECK_THROWS_AS(reader.ReadFiles("cbfbare_0001.cbf"), JFJochException); + remove("cbfbare_0001.cbf"); + } + + SECTION("a header number that does not parse is a malformed header, not a raw std throw") { + // The captures are character classes, not number grammars: "[\\d.eE+-]+" matches a bare ".". + // std::stod answers that with std::invalid_argument, which CanRead does not catch - so merely + // LOOKING at the file threw out of the format probe. + WriteRawMiniCBF("cbfbadnum_0001.cbf", + "# Pixel_size 172e-6 m x 172e-6 m\n# Wavelength . A\n" + "# Detector_distance 0.3 m\n# Count_cutoff 1000 counts\n", nx, ny); + CHECK_NOTHROW(JFJochCBFReader::CanRead("cbfbadnum_0001.cbf")); + CHECK_FALSE(JFJochCBFReader::CanRead("cbfbadnum_0001.cbf")); + remove("cbfbadnum_0001.cbf"); + } + + SECTION("no Count_cutoff does not mean every pixel is saturated") { + // SaturationLimitFromValue(0) is 1, so an absent line marked every pixel at or above one + // count as an overload and the integration accept gate then dropped the whole reflection. + WriteMiniCBFSweep("cbfnocut", "# Oscillation_axis OMEGA\n", nx, ny); + // ...rewrite frame 1 without the Count_cutoff line, keeping everything else. + WriteRawMiniCBF("cbfnocut_0001.cbf", + "# Detector: PILATUS3 6M, S/N 60-0119\n" + "# Pixel_size 172e-6 m x 172e-6 m\n" + "# Silicon sensor, thickness 0.000450 m\n" + "# Wavelength 0.96864 A\n# Detector_distance 0.33161 m\n" + "# Beam_xy (12.00, 8.00) pixels\n" + "# Start_angle 0 deg.\n# Angle_increment 0.1000 deg.\n", nx, ny); + JFJochCBFReader reader; + REQUIRE_NOTHROW(reader.ReadFiles("cbfnocut_0001.cbf")); + const auto x = reader.GetDataset()->experiment; + CHECK(x.GetSaturationLimit() > 1); + reader.Close(); + RemoveMiniCBFSweep("cbfnocut"); + } +}