v1.0.0-rc.166 (#76)

* `rugnux --mode calibration` writes `<prefix>.json` beside the `.poni`, whose `dataset_settings` member is a `jfjoch_broker` `dataset_settings` body as it stands.
* `rugnux` and `jfjoch_viewer` read PILATUS miniCBF sweeps natively, without conversion.
* Masters written by other facilities open, including Eiger 1.x and third-party NXmx variants.
* `rugnux` measures the beam centre on every run, and indexes with it when the file's value indexes nothing.
* A detector swung out on a 2theta arm is placed where the file says it stands, and the calibration can hold the tilt fixed.
* `rugnux` writes the unmerged MTZ by default, and a P1 merge beside it, so a wrong space group can be re-merged without reprocessing.
* Significant improvements to symmetry handling in `rugnux`: the lattice, the point group, the setting and the systematic absences.
* The `rugnux` report gives the resolution the CC1/2 fit reached, beside the range the reflections were written to.
* The `rugnux` report gives the twinning statistics measured before the space group was decided, beside the ones measured after.
* The `rugnux` report gives the strong-direction diffraction limit, and warns when CC1/2 is not monotone with resolution.
* `rugnux` ranks screw axes on the evidence their absences carry, rather than on how many control reflections a candidate happens to have.
* Twinning is no longer reported when the L-test contradicts it.
* The `rugnux` report gives the detector tilt, the measured tilt and the direct beam beside the beam centre, and a post-refined beam centre is judged against the run's own measurement rather than the file's.
* `--no-refine-tilt` holds the detector tilt at the value in the file, instead of zeroing it, when the calibration starts from the spots.
* The `jfjoch_viewer` grid scan view draws the cells in the proportion of the scan steps, so the map has the shape of the scanned area.

Reviewed-on: #76
Co-authored-by: Filip Leonarski <filip.leonarski@psi.ch>

tests/CalibrationTest.cpp

@@ -7,11 +7,17 @@
 #include <fstream>
 #include <map>
 #include <sstream>
+#include <algorithm>
 
 #include "../common/Definitions.h"
 #include "../common/JFJochMath.h"
+#include <nlohmann/json.hpp>
+
+#include "Powder_calibration_output.h"
+#include "Dataset_settings.h"
+#include "../image_analysis/geom_refinement/AssignSpotsToRings.h"
 #include "../image_analysis/geom_refinement/Calibrants.h"
-#include "../rugnux/RugnuxCalibration.h"
+#include "../image_analysis/geom_refinement/PowderCalibration.h"
 
 TEST_CASE("Calibrants_LookupIsCaseInsensitive", "[DetGeomCalib]") {
     CHECK(CalibrantRings("LaB6") == CalibrantRings("lab6"));
@@ -65,7 +71,7 @@ TEST_CASE("Calibrants_CentredStandardsOmitTheExtinctRings", "[DetGeomCalib]") {
 // Ice is the reason the calibrant abstraction is a ring list and not a UnitCell: its entries are
 // measured ring positions, and enumerating hkl from the hexagonal cell would add rings that are
 // systematically absent in P6_3/mmc.
-TEST_CASE("Calibrants_IceIsTheMeasuredRingList", "[DetGeomCalib]") {
+TEST_CASE("Calibrants_IceIsTheRingList", "[DetGeomCalib]") {
     const auto q = CalibrantRings("ice");
     REQUIRE(q.size() == ICE_RING_RES_A.size());
     CHECK(std::is_sorted(q.begin(), q.end()));
@@ -128,3 +134,283 @@ TEST_CASE("Calibration_PoniFileAxisConvention", "[DetGeomCalib]") {
                             + std::to_string(x.GetXPixelsNumConv()) + "]";
     CHECK(keys["Detector_config"].find(shape) != std::string::npos);
 }
+
+// The match window may never reach the neighbouring ring, for any calibrant. Where two rings are closer
+// together than twice the nominal window, a fixed window takes in the neighbour's flank - which the
+// rings path reads as this ring's background, and which the spots path (before it took the NEAREST ring)
+// resolved by assigning both to the lower-q one.
+TEST_CASE("Calibration_RingMatchWindowNeverReachesTheNeighbour", "[DetGeomCalib]") {
+    for (const auto &c : Calibrants()) {
+        const auto q = CalibrantRings(c.name);
+        REQUIRE(q.size() > 1);
+        for (size_t i = 0; i < q.size(); ++i) {
+            const float w = RingMatchWindow(q, i, RING_MATCH_Q_RECIPA);
+            CHECK(w <= RING_MATCH_Q_RECIPA);
+            CHECK(w > 0.0f);
+            if (i > 0)
+                CHECK(q[i] - w >= 0.5f * (q[i] + q[i - 1]) - 1e-6f);
+            if (i + 1 < q.size())
+                CHECK(q[i] + w <= 0.5f * (q[i] + q[i + 1]) + 1e-6f);
+        }
+    }
+}
+
+// ...and the clamp is not a no-op. Silver behenate's orders sit about 0.108 1/A apart and hexagonal ice
+// has rings inside 0.06, so both are narrowed below the nominal window - while LaB6, whose rings are
+// well separated at low q, keeps it. Without a standard that actually crowds, the test above would pass
+// on a clamp that never fired.
+TEST_CASE("Calibration_CrowdedStandardsNarrowTheWindow", "[DetGeomCalib]") {
+    auto narrowed = [](const std::string &name) {
+        const auto q = CalibrantRings(name);
+        size_t n = 0;
+        for (size_t i = 0; i < q.size(); ++i)
+            if (RingMatchWindow(q, i, RING_MATCH_Q_RECIPA) < RING_MATCH_Q_RECIPA) ++n;
+        return n;
+    };
+    CHECK(narrowed("agbh") > 0);
+    CHECK(narrowed("ice") > 0);
+    // The innermost LaB6 rings are more than 0.2 1/A apart, so nothing narrows them.
+    const auto lab6 = CalibrantRings("lab6");
+    CHECK(RingMatchWindow(lab6, 0, RING_MATCH_Q_RECIPA) == Catch::Approx(RING_MATCH_Q_RECIPA));
+}
+
+// A cell given with -C takes its absences from -S. That path is independent of the hand-written
+// ReflectionConditions the built-in table uses, so the two must agree where the standard's absences are
+// a property of its SYMMETRY - which is what says the gemmi route is safe to hand a user's cell.
+TEST_CASE("Calibration_SpaceGroupAbsencesMatchTheBuiltInConditions", "[DetGeomCalib]") {
+    struct Standard { std::string name; UnitCell cell; std::string hm; };
+    const std::vector<Standard> standards = {
+        {"lab6", UnitCell(LAB6_CELL_A, LAB6_CELL_A, LAB6_CELL_A, 90, 90, 90), "P m -3 m"},
+        {"ceo2", UnitCell(5.4115, 5.4115, 5.4115, 90, 90, 90),                "F m -3 m"}
+    };
+    for (const auto &s : standards) {
+        const gemmi::SpaceGroup *sg = gemmi::find_spacegroup_by_name(s.hm);
+        REQUIRE(sg != nullptr);
+        const auto from_sg = CalculateXtalRings(s.cell, *sg);
+        const auto from_table = CalibrantRings(s.name);
+        REQUIRE(from_sg.size() == from_table.size());
+        for (size_t i = 0; i < from_sg.size(); ++i)
+            CHECK(from_sg[i] == Catch::Approx(from_table[i]).epsilon(1e-6));
+    }
+}
+
+// Silicon is the case where they must NOT agree, and it is worth pinning because it bounds what -C -S
+// can do. Fd-3m's symmetry absences are only the F centring; silicon's further extinctions - 222 is the
+// first - come from its two-atom basis, i.e. from the structure factor and not from any symmetry
+// element, so gemmi cannot know them and reports 24 rings where the table's diamond condition gives 18.
+// The extra ones are exactly the all-even reflections with h+k+l not a multiple of 4. They do not move
+// the FIRST ring, so the calibration is not scaled wholesale - but they are rings carrying no intensity
+// offered to the matcher in the middle of the list, which is why --calibrant si still exists.
+TEST_CASE("Calibration_SpaceGroupCannotKnowStructureFactorAbsences", "[DetGeomCalib]") {
+    const UnitCell si(5.43102, 5.43102, 5.43102, 90, 90, 90);
+    const gemmi::SpaceGroup *sg = gemmi::find_spacegroup_by_name("F d -3 m");
+    REQUIRE(sg != nullptr);
+    const auto from_sg = CalculateXtalRings(si, *sg);
+    const auto from_table = CalibrantRings("si");
+    CHECK(from_sg.size() > from_table.size());
+    // The table's rings are a subset of the space group's - nothing is LOST by asking gemmi, only added.
+    for (const float q : from_table) {
+        const bool present = std::any_of(from_sg.begin(), from_sg.end(),
+                                         [q](float r) { return std::fabs(r - q) < 1e-4f; });
+        CHECK(present);
+    }
+    // ...and the first ring, the one the distance is seeded from, is the same either way.
+    CHECK(from_sg.front() == Catch::Approx(from_table.front()).epsilon(1e-6));
+}
+
+// Without -S the cell is taken as primitive, which for a centred standard is NOT the same list: the
+// face-centred absences are what move the first ring from 100 out to 111. The point of the test is that
+// the difference is real, so that "assumed primitive" in the log is a warning worth reading.
+TEST_CASE("Calibration_PrimitiveAssumptionDiffersForACentredCell", "[DetGeomCalib]") {
+    const UnitCell ceo2(5.4115, 5.4115, 5.4115, 90, 90, 90);
+    const gemmi::SpaceGroup *sg = gemmi::find_spacegroup_by_name("F m -3 m");
+    REQUIRE(sg != nullptr);
+    const auto centred = CalculateXtalRings(ceo2, *sg);
+    const auto primitive = CalculateXtalRings(ceo2);
+    REQUIRE(!centred.empty());
+    REQUIRE(!primitive.empty());
+    CHECK(primitive.size() > centred.size());
+    CHECK(centred.front() > primitive.front());
+}
+
+// The JSON a calibration writes has to BE a dataset_settings body, not merely resemble one. Its
+// "dataset_settings" member is fed straight into the model generated from broker/jfjoch_api.yaml, which
+// is the only definition of that schema this project has - so if the spec grows a field, renames one or
+// changes what it requires, this fails rather than a beamline discovering it at a POST.
+TEST_CASE("Calibration_JsonIsADatasetSettingsBody", "[DetGeomCalib]") {
+    DiffractionExperiment x(DetJF4M());
+    x.IncidentEnergy_keV(12.4f).BeamX_pxl(1000.0f).BeamY_pxl(1050.0f).DetectorDistance_mm(150.0f);
+
+    CalibrationResult result;
+    result.geometry = x.GetDiffractionGeometry();
+    result.geometry.BeamX_pxl(1001.25f).BeamY_pxl(1049.5f).DetectorDistance_mm(151.5f)
+                   .PoniRot1_rad(0.01f).PoniRot2_rad(-0.02f);
+    result.ring_points = 321;
+    result.rms_radial_pxl = 0.42;
+    result.tilt_refined = true;
+    result.tilt_significance = 17.5f;
+    result.header_distance_mm = 150.0f;
+
+    const std::string path = "calibration_json_test.json";
+    WriteCalibrationJson(path, x, result, "lab6", "rings");
+    std::ifstream in(path);
+    REQUIRE(in.good());
+    nlohmann::json j;
+    in >> j;
+    in.close();
+    std::remove(path.c_str());
+
+    REQUIRE(j.contains("dataset_settings"));
+    const auto &settings = j.at("dataset_settings");
+
+    // The WHOLE file is a calibration_output, not just its geometry member - so the quality block a
+    // reader needs in order to tell a calibration that worked from one that did not is part of the
+    // published contract too, and a python client can decode the file without knowing anything else.
+    org::openapitools::server::model::Powder_calibration_output output;
+    REQUIRE_NOTHROW(from_json(j, output));
+    std::stringstream output_msg;
+    CHECK(output.validate(output_msg));
+    CHECK(output.getCalibration().getRingPoints() == 321);
+    CHECK(output.getCalibration().getMethod() == "rings");
+    CHECK(output.getCalibration().isTiltRefined());
+
+    // Every key is a property the schema knows, and the four it requires are all there.
+    org::openapitools::server::model::Dataset_settings model;
+    REQUIRE_NOTHROW(from_json(settings, model));
+    std::stringstream msg;
+    CHECK(model.validate(msg));
+
+    CHECK(model.getBeamXPxl() == Catch::Approx(1001.25));
+    CHECK(model.getBeamYPxl() == Catch::Approx(1049.5));
+    CHECK(model.getDetectorDistanceMm() == Catch::Approx(151.5));
+    CHECK(model.getIncidentEnergyKeV() == Catch::Approx(12.4));
+    // beam_x_pxl is the PONI, so it must be the fitted PONI and NOT the direct beam - those differ by
+    // distance*tan(tilt)/pixel here, and writing the wrong one would move a beamline's geometry.
+    const auto [direct_x, direct_y] = result.geometry.GetDirectBeam_pxl();
+    CHECK(std::abs(direct_x - model.getBeamXPxl()) > 0.5f);
+    CHECK(j.at("calibration").at("direct_beam_x_pxl").get<double>() == Catch::Approx(direct_x).margin(0.01));
+
+    // A tilted geometry carries all three rotations, because a body without them states a FLAT
+    // detector rather than an unstated one.
+    CHECK(model.getPoniRot1Rad() == Catch::Approx(0.01).margin(1e-6));
+    CHECK(model.getPoniRot2Rad() == Catch::Approx(-0.02).margin(1e-6));
+    CHECK(settings.contains("poni_rot3_rad"));
+}
+
+// ...and an untilted result leaves the rotations out altogether, which means the same thing: the API's
+// own default for each is 0.0. The test is here so the two branches cannot drift apart.
+TEST_CASE("Calibration_JsonOmitsTheRotationsWhenTheyAreZero", "[DetGeomCalib]") {
+    DiffractionExperiment x(DetJF4M());
+    x.IncidentEnergy_keV(12.4f).DetectorDistance_mm(150.0f);
+
+    CalibrationResult result;
+    result.geometry = x.GetDiffractionGeometry();
+    result.geometry.PoniRot1_rad(0.0f).PoniRot2_rad(0.0f).PoniRot3_rad(0.0f);
+    result.tilt_refined = false;
+
+    const std::string path = "calibration_json_flat_test.json";
+    WriteCalibrationJson(path, x, result, "lab6", "rings");
+    std::ifstream in(path);
+    nlohmann::json j;
+    in >> j;
+    in.close();
+    std::remove(path.c_str());
+
+    const auto &settings = j.at("dataset_settings");
+    CHECK_FALSE(settings.contains("poni_rot1_rad"));
+    CHECK_FALSE(settings.contains("poni_rot2_rad"));
+    CHECK_FALSE(settings.contains("poni_rot3_rad"));
+    org::openapitools::server::model::Dataset_settings model;
+    REQUIRE_NOTHROW(from_json(settings, model));
+    std::stringstream msg;
+    CHECK(model.validate(msg));
+}
+
+// The gate. A calibration is run because the input file's geometry is in doubt, so a fit that hands
+// part of that file back has answered nothing - and it looks exactly like a fit that worked in every
+// number a caller reads. These four cases are the whole rule: it turns on where a value came FROM, not
+// on how large any residual is, so there is nothing here calibrated on a population.
+TEST_CASE("Calibration_ADeclinedTiltPinnedAtTheFilesOwnIsNotAMeasurement", "[DetGeomCalib]") {
+    DiffractionExperiment x(DetJF4M());
+    x.IncidentEnergy_keV(12.4f).DetectorDistance_mm(190.0f);
+    // The tilt an in-house master file states, which is hardcoded and not measured on that instrument.
+    DiffractionGeometry header = x.GetDiffractionGeometry();
+    header.PoniRot1_rad(-0.0013963f).PoniRot2_rad(-0.0038397f);
+
+    CalibrationResult result;
+    result.geometry = header;                 // what the pinned refit hands back: the file's own tilt
+    result.uncertainty.valid = true;
+    result.tilt_refined = false;
+    result.tilt_significance = 2.93f;
+
+    JudgeCalibration(result, header, true);
+    CHECK_FALSE(result.converged);
+    CHECK_FALSE(result.reason.empty());
+
+    // ...and the same fit is honest where the file's tilt is zero, because then declining the tilt and
+    // reporting the file's value say the same thing. This is the long-distance regime the significance
+    // test was put in for, and it must keep passing.
+    DiffractionGeometry flat = x.GetDiffractionGeometry();
+    CalibrationResult declined_at_zero;
+    declined_at_zero.geometry = flat;
+    declined_at_zero.uncertainty.valid = true;
+    declined_at_zero.tilt_refined = false;
+    declined_at_zero.tilt_significance = 0.1f;
+    JudgeCalibration(declined_at_zero, flat, true);
+    CHECK(declined_at_zero.converged);
+
+    // ...and --no-refine-tilt over the same file is not this failure either: there the user asked for
+    // the file's tilt to be held, which is a stated choice and not a silent substitution.
+    CalibrationResult held;
+    held.geometry = header;
+    held.uncertainty.valid = true;
+    held.tilt_refined = false;
+    JudgeCalibration(held, header, false);
+    CHECK(held.converged);
+
+    // A covariance that never conditioned is the other way out with nothing measured behind it: the fit
+    // sits where some direction in parameter space costs it nothing, so it cannot say what it fixed.
+    CalibrationResult degenerate;
+    degenerate.geometry = x.GetDiffractionGeometry();
+    degenerate.uncertainty.valid = false;
+    degenerate.tilt_refined = true;
+    JudgeCalibration(degenerate, x.GetDiffractionGeometry(), true);
+    CHECK_FALSE(degenerate.converged);
+}
+
+// The verdict has to reach whatever reads the file next, not only the terminal - the .poni is withheld
+// when it is false, so the JSON is the only place a caller can learn that a calibration ran and failed.
+TEST_CASE("Calibration_JsonCarriesTheVerdict", "[DetGeomCalib]") {
+    DiffractionExperiment x(DetJF4M());
+    x.IncidentEnergy_keV(12.4f).DetectorDistance_mm(190.0f);
+
+    CalibrationResult result;
+    result.geometry = x.GetDiffractionGeometry();
+    result.converged = false;
+    result.reason = "the tilt was declined and pinned at the input file's value";
+
+    const std::string path = "calibration_json_verdict_test.json";
+    WriteCalibrationJson(path, x, result, "lab6", "rings");
+    std::ifstream in(path);
+    REQUIRE(in.good());
+    nlohmann::json j;
+    in >> j;
+    in.close();
+    std::remove(path.c_str());
+
+    CHECK(j.at("calibration").at("converged").get<bool>() == false);
+    CHECK(j.at("calibration").at("not_converged_reason").get<std::string>() == result.reason);
+
+    // A converged fit says so too, rather than saying nothing - absence would be read as an older file
+    // that could not have known, which is exactly the ambiguity this closes.
+    result.converged = true;
+    result.reason.clear();
+    WriteCalibrationJson(path, x, result, "lab6", "rings");
+    std::ifstream in2(path);
+    nlohmann::json j2;
+    in2 >> j2;
+    in2.close();
+    std::remove(path.c_str());
+    CHECK(j2.at("calibration").at("converged").get<bool>() == true);
+    CHECK_FALSE(j2.at("calibration").contains("not_converged_reason"));
+}