diff --git a/common/DiffractionExperiment.cpp b/common/DiffractionExperiment.cpp index 79d3f29e..8f51fbb7 100644 --- a/common/DiffractionExperiment.cpp +++ b/common/DiffractionExperiment.cpp @@ -1504,8 +1504,10 @@ std::vector DiffractionExperiment::BuildTransformationCh if (const auto smargon = dataset.GetSmargonPosition()) { const auto n = static_cast(std::max(image_num, 1)); add(DetectorTransformation("chi", TransformationType::Rotation, smargon->chi_axis) + .EquipmentComponent("smargon") .Values(std::vector(n, smargon->chi_deg))); add(DetectorTransformation("phi", TransformationType::Rotation, smargon->phi_axis) + .EquipmentComponent("smargon") .Values(std::vector(n, smargon->phi_deg))); } diff --git a/reader/HDF5MetadataSource.cpp b/reader/HDF5MetadataSource.cpp index 5d79bbb8..d064078b 100644 --- a/reader/HDF5MetadataSource.cpp +++ b/reader/HDF5MetadataSource.cpp @@ -609,8 +609,23 @@ HDF5MetadataSource::OpenResult HDF5MetadataSource::Open(const std::string &filen // recorded as "phi" back as stills, silently. Prefer an axis that actually turns; fall back // to a stationary one, which still says where the head was. if (master_file->Exists("/entry/sample/transformations")) { + // A Smargon chi/phi is tagged with equipment_component - it is a head position, not the + // spindle. Recognised by that tag and not by name: phi is an ordinary spindle name in MX, + // so a file from anywhere else must not have its rotation axis read back as a head + // position, nor its spindle mistaken for one here. + auto is_smargon_axis = [this](const std::string &name) { + const std::string dname = "/entry/sample/transformations/" + name; + if (!master_file->Exists(dname)) + return false; + HDF5DataSet axis(*master_file, dname); + return axis.AttrExists("equipment_component") + && (axis.ReadAttrStr("equipment_component") == "smargon"); + }; + std::optional stationary; for (const auto &name: master_file->FindLeafs("/entry/sample/transformations")) { + if (is_smargon_axis(name)) + continue; auto axis = ReadAxis(master_file.get(), name); if (!axis.has_value()) continue; @@ -628,8 +643,11 @@ HDF5MetadataSource::OpenResult HDF5MetadataSource::Open(const std::string &filen // chi and phi are ordinary stationary axes in the file; the settings still keep them in // their own Smargon field, so put them back there. Without this a re-opened file lost // the head position entirely - nothing in reader/ read it. - const auto chi = ReadAxis(master_file.get(), "chi"); - const auto phi = ReadAxis(master_file.get(), "phi"); + std::optional chi, phi; + if (is_smargon_axis("chi")) + chi = ReadAxis(master_file.get(), "chi"); + if (is_smargon_axis("phi")) + phi = ReadAxis(master_file.get(), "phi"); if (chi.has_value() || phi.has_value()) { SmargonPosition smargon; if (chi.has_value()) { diff --git a/tests/JFJochReaderTest.cpp b/tests/JFJochReaderTest.cpp index 69d2bca3..783cfad6 100644 --- a/tests/JFJochReaderTest.cpp +++ b/tests/JFJochReaderTest.cpp @@ -2995,3 +2995,130 @@ TEST_CASE("JFJochReader_TransformationChain_SentAndBuilt", "[HDF5][Full]") { remove("test_chain_sent_master.h5"); REQUIRE(H5Fget_obj_count(H5F_OBJ_ALL, H5F_OBJ_ALL) == 0); } + +// A head position is not a sweep, and it is not the spindle either. Both properties are carried by +// the file itself - the axis length says how many images there are, the equipment_component tag says +// what the axis is - so both are checked here on the file, not through the reader: the reader alone +// cannot see a shape or an attribute it never looks at. +TEST_CASE("JFJochReader_Smargon_StillIsNotOneImage", "[HDF5][Full]") { + DiffractionExperiment x(DetJF(1)); + x.ImagesPerTrigger(5).OverwriteExistingFiles(true).FilePrefix("test_smargon"); + x.BeamX_pxl(100).BeamY_pxl(200).DetectorDistance_mm(150) + .IncidentEnergy_keV(WVL_1A_IN_KEV).PixelSigned(false).BitDepthImage(16) + .FrameTime(std::chrono::microseconds(500), std::chrono::microseconds(10)); + x.Smargon(SmargonPosition{.phi_deg = -7.25f, .chi_deg = 12.5f}); + REQUIRE(!x.GetGoniometer().has_value()); + + RegisterHDF5Filter(); + std::vector image(x.GetPixelsNum(), 0); + + StartMessage start_message; + x.FillMessage(start_message); + FileWriter file_set(start_message); + DataMessage message{}; + for (int i = 0; i < x.GetImageNum(); i++) { + message.image = CompressedImage(image, x.GetXPixelsNum(), x.GetYPixelsNum()); + message.number = i; + REQUIRE_NOTHROW(file_set.WriteHDF5(message)); + } + EndMessage end_message; + end_message.max_image_number = x.GetImageNum(); + file_set.WriteHDF5(end_message); + file_set.Finalize(); + + { + HDF5ReadOnlyFile master("test_smargon_master.h5"); + + // One entry per image. A reader takes the image count from the innermost axis of the sample + // chain when no axis varies; as scalars these read back as a single image. + CHECK(master.GetDimension("/entry/sample/transformations/chi") + == std::vector{static_cast(x.GetImageNum())}); + CHECK(master.GetDimension("/entry/sample/transformations/phi") + == std::vector{static_cast(x.GetImageNum())}); + CHECK(master.ReadVector("/entry/sample/transformations/phi") + == std::vector(x.GetImageNum(), -7.25)); + + // Tagged, so neither is mistaken for the spindle - and so a phi from anywhere else is not + // mistaken for a head position. + HDF5DataSet chi(master, "/entry/sample/transformations/chi"); + HDF5DataSet phi(master, "/entry/sample/transformations/phi"); + REQUIRE(chi.AttrExists("equipment_component")); + REQUIRE(phi.AttrExists("equipment_component")); + CHECK(chi.ReadAttrStr("equipment_component") == "smargon"); + CHECK(phi.ReadAttrStr("equipment_component") == "smargon"); + } + + const auto read = [](const std::string &prefix) { + JFJochHDF5Reader reader; + reader.ReadFile(prefix + "_master.h5"); + return reader.GetDataset()->experiment; + }; + const auto read_back = read("test_smargon"); + + // chi is the alphabetically first stationary axis in the file; it must not become the spindle. + CHECK(!read_back.GetGoniometer().has_value()); + REQUIRE(read_back.GetDatasetSettings().GetSmargonPosition().has_value()); + CHECK(read_back.GetDatasetSettings().GetSmargonPosition()->chi_deg == Catch::Approx(12.5f).margin(1e-3)); + CHECK(read_back.GetDatasetSettings().GetSmargonPosition()->phi_deg == Catch::Approx(-7.25f).margin(1e-3)); + + remove("test_smargon_master.h5"); + remove("test_smargon_data_000001.h5"); + REQUIRE(H5Fget_obj_count(H5F_OBJ_ALL, H5F_OBJ_ALL) == 0); +} + +// phi is an ordinary spindle name in MX. A file whose rotation axis is called phi carries no +// equipment_component, so it stays the spindle and no head position is invented from it - which also +// means the file can be written back out, instead of colliding on a second dataset called phi. +TEST_CASE("JFJochReader_Goniometer_NamedPhiIsNotSmargon", "[HDF5][Full]") { + DiffractionExperiment x(DetJF(1)); + x.ImagesPerTrigger(5).OverwriteExistingFiles(true).FilePrefix("test_phispindle"); + x.BeamX_pxl(100).BeamY_pxl(200).DetectorDistance_mm(150) + .IncidentEnergy_keV(WVL_1A_IN_KEV).PixelSigned(false).BitDepthImage(16) + .FrameTime(std::chrono::microseconds(500), std::chrono::microseconds(10)); + x.Goniometer(GoniometerAxis("phi", 30, 0.2f, Coord(0,-1,0), {})); + + RegisterHDF5Filter(); + std::vector image(x.GetPixelsNum(), 0); + + StartMessage start_message; + x.FillMessage(start_message); + FileWriter file_set(start_message); + DataMessage message{}; + for (int i = 0; i < x.GetImageNum(); i++) { + message.image = CompressedImage(image, x.GetXPixelsNum(), x.GetYPixelsNum()); + message.number = i; + REQUIRE_NOTHROW(file_set.WriteHDF5(message)); + } + EndMessage end_message; + end_message.max_image_number = x.GetImageNum(); + file_set.WriteHDF5(end_message); + file_set.Finalize(); + + const auto read = [](const std::string &prefix) { + JFJochHDF5Reader reader; + reader.ReadFile(prefix + "_master.h5"); + return reader.GetDataset()->experiment; + }; + const auto read_back = read("test_phispindle"); + + REQUIRE(read_back.GetGoniometer().has_value()); + CHECK(read_back.GetGoniometer()->GetName() == "phi"); + CHECK(read_back.GetGoniometer()->GetStart_deg() == Catch::Approx(30).margin(1e-3)); + CHECK(!read_back.GetDatasetSettings().GetSmargonPosition().has_value()); + + // Writing what was read must not try to create phi a second time. + DiffractionExperiment rewrite = read_back; + rewrite.FilePrefix("test_phispindle_out").OverwriteExistingFiles(true); + StartMessage out_start; + rewrite.FillMessage(out_start); + FileWriter out(out_start); + EndMessage out_end; + out_end.max_image_number = rewrite.GetImageNum(); + REQUIRE_NOTHROW(out.WriteHDF5(out_end)); + REQUIRE_NOTHROW(out.Finalize()); + + remove("test_phispindle_master.h5"); + remove("test_phispindle_data_000001.h5"); + remove("test_phispindle_out_master.h5"); + REQUIRE(H5Fget_obj_count(H5F_OBJ_ALL, H5F_OBJ_ALL) == 0); +} diff --git a/writer/HDF5NXmx.cpp b/writer/HDF5NXmx.cpp index 281e8e52..36418408 100644 --- a/writer/HDF5NXmx.cpp +++ b/writer/HDF5NXmx.cpp @@ -769,12 +769,12 @@ void NXmx::Sample(const StartMessage &start, const EndMessage &end) { const auto n = static_cast(std::max(end.max_image_number, 1)); SaveVector(transformations, "chi", std::vector(n, start.smargon_position->chi_deg))-> - Transformation("deg", depends_on, "", "", "rotation", + Transformation("deg", depends_on, "", "smargon", "rotation", {start.smargon_position->chi_axis.x, start.smargon_position->chi_axis.y, start.smargon_position->chi_axis.z}, {0, 0, 0}, ""); depends_on = "/entry/sample/transformations/chi"; SaveVector(transformations, "phi", std::vector(n, start.smargon_position->phi_deg))-> - Transformation("deg", depends_on, "", "", "rotation", + Transformation("deg", depends_on, "", "smargon", "rotation", {start.smargon_position->phi_axis.x, start.smargon_position->phi_axis.y, start.smargon_position->phi_axis.z}, {0, 0, 0}, ""); depends_on = "/entry/sample/transformations/phi"; diff --git a/writer/HDF5Objects.cpp b/writer/HDF5Objects.cpp index 1dac248a..10847e37 100644 --- a/writer/HDF5Objects.cpp +++ b/writer/HDF5Objects.cpp @@ -534,6 +534,10 @@ std::vector HDF5Object::ReadAttrVec(const std::string &name) { return ret; } +bool HDF5Object::AttrExists(const std::string &name) { + return H5Aexists(id, name.c_str()) > 0; +} + std::string HDF5Object::ReadAttrStr(const std::string &name) { hid_t attr_id = H5Aopen(id, name.c_str(), H5P_DEFAULT); if (attr_id < 0) diff --git a/writer/HDF5Objects.h b/writer/HDF5Objects.h index 02c32d1b..85250e9c 100644 --- a/writer/HDF5Objects.h +++ b/writer/HDF5Objects.h @@ -138,6 +138,7 @@ public: HDF5Object& Attr(const std::string& name, const std::vector &val); std::vector ReadAttrVec(const std::string& name); + bool AttrExists(const std::string &name); std::string ReadAttrStr(const std::string &name); double ReadAttrDouble(const std::string &name); int64_t ReadAttrInt(const std::string &name);