From 1ea6ba25be60f424afb62d208dfe85a29ce8f0fd Mon Sep 17 00:00:00 2001 From: Filip Leonarski Date: Sat, 29 Aug 2026 15:47:41 +0200 Subject: [PATCH] reader: open a master that states its goniometer axes in one place and their angles in another A hybrid master carries an NXmx /entry/sample/transformations group holding one EMPTY SUBGROUP per axis - the direction as a vector attribute, no NX_class, no units, no angles - beside a legacy /entry/sample/goniometer group holding all the angles. Opening it failed outright with "Cannot open HDF5 dataset /entry/sample/transformations/omega": the existing legacy fallback keys off Exists("/entry/sample/transformations"), which is true here, so it never fired, and the axis stub was then opened as if it were the angle dataset. Present is not the same as usable. GoniometerGroup now takes transformations only if it holds at least one DATASET, and ReadAxis asks IsDataSet rather than Exists, so a member that is not a dataset can no longer be read as one. HDF5Object gains that predicate, in the style of the neighbouring Exists. The stub is the load-bearing half, not merely the thing that crashed. In the legacy branch, before falling back to the assumed (-1,0,0), the reader now looks for the NXmx stub and takes its stated vector. With it the axis is (0,-1,0) and the run indexes 60/60 validation frames; with the stub deleted the assumption applies and the same file indexes 0/60 on both schemes and both signs, and the run stops with no lattice. So without this half the fix would have turned "cannot open" into "found no lattice" - a differently shaped failure, not a success. The direction stated here is 90 degrees from the assumption, not merely its negation, which the rotation first pass could have recovered on its own. The vector size check moved out of the first branch so it now covers every path that produces one. Verified: the file processes end to end, 100% of frames indexed, cubic cell 105.87 against a deposited 105.88 (0.009%), space group reported as I23 or I213 - correctly refusing to choose, since the reflections that separate them are extinguished by the I-centring and were never measured. A both-layout master reprocesses unchanged (100% indexed, P212121). [HDF5] passes: 2194 assertions in 91 cases. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Lc5JG6kJqZoCWaoZ43JGTW --- docs/CHANGELOG.md | 1 + reader/HDF5MetadataSource.cpp | 45 +++++++++++++++++++++++++---------- writer/HDF5Objects.cpp | 11 +++++++++ writer/HDF5Objects.h | 1 + 4 files changed, 46 insertions(+), 12 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 1625f4509..312df2cf6 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -1,6 +1,7 @@ # Changelog ## 1.0.0 ### 1.0.0-rc.166 +* A master that states its goniometer axis directions in an NXmx `transformations` group but keeps the angles in the legacy `goniometer` group now opens, and takes the axis direction from what the file states rather than assuming it. * A run whose indexer could not run at all - an exhausted GPU, most often - reports that, instead of reporting that no lattice was found and advising the beam centre be checked. * A run that exhausts GPU or host memory stops with an error instead of skipping the images it could not process and finishing as if they had never been there. * Each scaling pass of a de-novo rotation run runs the number of scaling iterations it was asked for, instead of continuing the previous pass's. A run given its space group with `-S` is unaffected. diff --git a/reader/HDF5MetadataSource.cpp b/reader/HDF5MetadataSource.cpp index fa47c4232..e2897b3e1 100644 --- a/reader/HDF5MetadataSource.cpp +++ b/reader/HDF5MetadataSource.cpp @@ -128,9 +128,17 @@ int64_t ReadIntWithLegacyFallback(HDF5Object &file, const std::string &nxmx, con // them in /entry/sample/goniometer and wrote no transformation_type and no vector on them. Getting // this one wrong is not a missing value but a WRONG ANSWER: a goniometer is only ever set from this // group, so a file whose axes are somewhere else is read as stills, silently. +// +// A hybrid file has both: an NXmx transformations group holding one empty subgroup per axis, which +// states the direction and nothing else, beside a legacy goniometer group holding all the angles. +// So present is not the same as usable - transformations is the angle source only if it holds an +// axis dataset, and an axis is always a dataset. std::string GoniometerGroup(HDF5Object &file) { - if (file.Exists("/entry/sample/transformations")) - return "/entry/sample/transformations"; + if (file.Exists("/entry/sample/transformations")) { + for (const auto &name: file.FindLeafs("/entry/sample/transformations")) + if (file.IsDataSet("/entry/sample/transformations/" + name)) + return "/entry/sample/transformations"; + } if (file.Exists("/entry/sample/goniometer")) return "/entry/sample/goniometer"; return {}; @@ -1186,7 +1194,8 @@ std::optional HDF5MetadataSource::ReadAxis(HDF5Object *file, con const std::string &group) { std::string dname = group + "/" + name; - if (!file->Exists(dname)) + // Not a dataset, not an axis: a hybrid file keeps a bare subgroup here for the direction alone. + if (!file->IsDataSet(dname)) return {}; @@ -1237,21 +1246,33 @@ std::optional HDF5MetadataSource::ReadAxis(HDF5Object *file, con std::vector axis_vec; if (dataset.AttrExists("vector")) { axis_vec = dataset.ReadAttrVec("vector"); - if (axis_vec.size() != 3) - throw JFJochException(JFJochExceptionCategory::InputParameterInvalid, - dname + " Vector must have 3 elements"); } else if (legacy_group) { - // Firmware 1.x stored no direction at all. Assume the one every DECTRIS master since has - // written, and say so - a wrong guess here does not index, so it is visible rather than - // silent, and the rotation first pass will try the opposite sign anyway. - axis_vec = {-1.0, 0.0, 0.0}; - Logger("HDF5Reader").Warning("{} carries no axis direction (pre-NXmx layout); assuming " - "(-1,0,0), the direction current DECTRIS masters write", dname); + // The angles carry no direction here, but a hybrid file still states one next door: an + // NXmx-shaped subgroup /entry/sample/transformations/AXIS, holding the vector attribute + // and no angles. That is the file speaking, so it beats the assumption below. + const std::string nxmx_axis = "/entry/sample/transformations/" + name; + if (file->Exists(nxmx_axis) && !file->IsDataSet(nxmx_axis)) { + HDF5Group nxmx_group(*file, nxmx_axis); + if (nxmx_group.AttrExists("vector")) + axis_vec = nxmx_group.ReadAttrVec("vector"); + } + if (axis_vec.empty()) { + // Firmware 1.x stored no direction at all. Assume the one every DECTRIS master since has + // written, and say so - a wrong guess here does not index, so it is visible rather than + // silent, and the rotation first pass will try the opposite sign anyway. + axis_vec = {-1.0, 0.0, 0.0}; + Logger("HDF5Reader").Warning("{} carries no axis direction (pre-NXmx layout); assuming " + "(-1,0,0), the direction current DECTRIS masters write", dname); + } } else { throw JFJochException(JFJochExceptionCategory::InputParameterInvalid, dname + " has no vector attribute"); } + if (axis_vec.size() != 3) + throw JFJochException(JFJochExceptionCategory::InputParameterInvalid, + dname + " Vector must have 3 elements"); + Coord axis(axis_vec[0], axis_vec[1], axis_vec[2]); GoniometerAxis g_axis(name, start, incr, axis, {}); if (!end.empty()) diff --git a/writer/HDF5Objects.cpp b/writer/HDF5Objects.cpp index 382cfab5e..ec9d5643b 100644 --- a/writer/HDF5Objects.cpp +++ b/writer/HDF5Objects.cpp @@ -1077,6 +1077,17 @@ bool HDF5Object::Exists(const std::string &name) const { return false; } +bool HDF5Object::IsDataSet(const std::string &name) const { + hid_t dataset_id; + H5E_BEGIN_TRY { + dataset_id = H5Dopen(GetID(), name.c_str(), H5P_DEFAULT); + } H5E_END_TRY; + if (dataset_id < 0) + return false; + H5Dclose(dataset_id); + return true; +} + herr_t func(hid_t group, const char *name, const H5L_info2_t *info, void *op_data) { auto ret = reinterpret_cast *>(op_data); ret->emplace_back(name); diff --git a/writer/HDF5Objects.h b/writer/HDF5Objects.h index 41fa261f0..741f3172d 100644 --- a/writer/HDF5Objects.h +++ b/writer/HDF5Objects.h @@ -182,6 +182,7 @@ public: const std::vector& start, const std::vector& size); bool Exists(const std::string& name) const; + bool IsDataSet(const std::string& name) const; bool IsExternalLink(const std::string& name) const; std::string GetLinkedFileName(const std::string& name) const; std::vector FindLeafs(const std::string &name) const;