diff --git a/image_analysis/rotation_indexer/RotationIndexer.cpp b/image_analysis/rotation_indexer/RotationIndexer.cpp index 2a25e8ec7..bb177c12c 100644 --- a/image_analysis/rotation_indexer/RotationIndexer.cpp +++ b/image_analysis/rotation_indexer/RotationIndexer.cpp @@ -9,7 +9,6 @@ #include "../lattice_search/LatticeSearch.h" #include "../indexing/MultiLatticeSearch.h" -#include #include #include #include @@ -255,7 +254,7 @@ void RotationIndexer::RunIndexing() { if (k == key) kept = outcome; } - if (kept && !std::getenv("RUGNUX_VERIFY_FIRST_PASS_MEMO")) { + if (kept && !verify_memo) { axis_ = kept->axis; updated_geom_ = kept->updated_geom; search_result_ = kept->search_result; @@ -272,7 +271,7 @@ void RotationIndexer::RunIndexing() { const IndexingOutcome outcome = Outcome(); if (kept) { if (!SameOutcome(*kept, outcome)) - throw std::runtime_error("RotationIndexer: the indexing memo differs from a recomputation"); + throw MemoMismatch("RotationIndexer: the indexing memo differs from a recomputation"); return; } auto &memo = Memo(); diff --git a/image_analysis/rotation_indexer/RotationIndexer.h b/image_analysis/rotation_indexer/RotationIndexer.h index 42cc86198..49bf1e0c7 100644 --- a/image_analysis/rotation_indexer/RotationIndexer.h +++ b/image_analysis/rotation_indexer/RotationIndexer.h @@ -6,6 +6,7 @@ #include #include #include +#include #include "../../common/DiffractionSpot.h" #include "../../common/DiffractionExperiment.h" @@ -38,6 +39,12 @@ struct RotationIndexerResult { std::optional tilt_walk; }; +// What a memo verification throws where a reused result differs from its recomputation - its own +// type, so that no pass that catches a failure and carries on can take it for an ordinary one. +struct MemoMismatch : std::runtime_error { + using std::runtime_error::runtime_error; +}; + class RotationIndexer { public: // RunIndexing is deterministic in what it reads - the accumulated spots and their angles, the @@ -45,8 +52,8 @@ public: // of the pool it indexes with - and a rotation run asks the same question again and again (the // canonical pass repeats its rotation-scale probe, a probe repeats its pass's rescue ladder). // So what it computes is kept, process-wide, under exactly those inputs, and a second - // RotationIndexer asking with the same inputs takes the answer. RUGNUX_VERIFY_FIRST_PASS_MEMO - // recomputes anyway and throws on any difference. + // RotationIndexer asking with the same inputs takes the answer. VerifyMemo(true) recomputes anyway + // and throws on any difference. struct IndexingOutcome { std::optional axis; DiffractionGeometry updated_geom; @@ -68,6 +75,7 @@ private: // caller lowers it to seed the search on the strongest few, exactly as the per-frame indexer // escalates 30 -> 80 -> all, for a pattern where the deep list is mostly not this crystal. size_t max_spots_per_image = DEFAULT_MAX_SPOTS_PER_IMAGE; + bool verify_memo = false; const bool index_ice_rings; const bool real_time; // see the constructor @@ -108,6 +116,8 @@ public: RotationIndexer(const DiffractionExperiment& x, IndexerThreadPool& indexer, bool real_time = false); // Keep only this many spots per image in the accumulated cloud (strongest, non-ring first). void MaxSpotsPerImage(size_t n) { max_spots_per_image = n > 0 ? n : DEFAULT_MAX_SPOTS_PER_IMAGE; } + // Recompute on a memo hit and throw if the two differ - the tests' check that the memo is exact. + void VerifyMemo(bool v) { verify_memo = v; } // angle_deg is the image's mid-exposure rotation angle; if omitted, the goniometer angle at // `image` is used (only valid when `image` is the goniometer's own image index). void ProcessImage(int64_t image, const std::vector& spots, diff --git a/rugnux/Rugnux.cpp b/rugnux/Rugnux.cpp index d5be30ec0..c66cd7429 100644 --- a/rugnux/Rugnux.cpp +++ b/rugnux/Rugnux.cpp @@ -148,6 +148,9 @@ namespace { // has quietly changed. Fail the run instead, and let the operator free the card and repeat it. bool IsFatalResourceError(const std::exception &e) { if (dynamic_cast(&e)) return true; + // Not a resource, but just as fatal: a memo verification (verify_first_pass_memo) that found a + // difference must end the run, not pass for a probe that scored nothing. + if (dynamic_cast(&e)) return true; const auto *jf = dynamic_cast(&e); return jf != nullptr && (jf->Category() == JFJochExceptionCategory::GPUCUDAError || jf->Category() == JFJochExceptionCategory::MemAllocFailed); @@ -3397,7 +3400,7 @@ ProcessResult Rugnux::RunPipeline(RugnuxObserver *observer, bool write_output, b // detector. Nothing between here and the first pass changes what the key reads. if (full && indexing_probe_only_ && !force_rotation_result_.has_value() && !config_.forced_rotation_lattice.has_value() && config_.rotation_indexing && config_.two_pass_rotation - && !std::getenv("RUGNUX_VERIFY_FIRST_PASS_MEMO")) { + && !config_.verify_first_pass_memo) { const std::vector first_pass_key = FirstPassInputKey(); const auto memo = std::find_if(first_pass_memo_.begin(), first_pass_memo_.end(), [&](const FirstPassMemo &m) { return m.key == first_pass_key; }); @@ -3579,7 +3582,7 @@ ProcessResult Rugnux::RunPipeline(RugnuxObserver *observer, bool write_output, b logger.Info("Rotation indexer lattice forced externally - skipping first pass"); } else if (full && config_.rotation_indexing && config_.two_pass_rotation) { // An indexing probe over inputs a first pass has already been run on returned above, except - // under RUGNUX_VERIFY_FIRST_PASS_MEMO, which runs the pass and compares. + // under verify_first_pass_memo, which runs the pass and compares. const std::vector first_pass_key = FirstPassInputKey(); std::optional memo_hit; const auto memo = std::find_if(first_pass_memo_.begin(), first_pass_memo_.end(), @@ -3668,7 +3671,6 @@ ProcessResult Rugnux::RunPipeline(RugnuxObserver *observer, bool write_output, b }; int prefetch_batches = 0; double prefetch_time_s = 0.0; - const bool verify_spots = std::getenv("RUGNUX_VERIFY_FIRST_PASS_MEMO") != nullptr; const auto prefetch_spots = [&](const std::vector &ordinals) { // What an earlier pass already found under the same key (first_pass_spots_) is taken from // there. @@ -3685,7 +3687,7 @@ ProcessResult Rugnux::RunPipeline(RugnuxObserver *observer, bool write_output, b std::vector wanted; for (const int ordinal : ordinals) if (!spot_cache.contains(ordinal)) { - if (const auto it = kept.find(ordinal); it != kept.end() && !verify_spots) + if (const auto it = kept.find(ordinal); it != kept.end() && !config_.verify_first_pass_memo) spot_cache.emplace(ordinal, it->second); else wanted.push_back(ordinal); @@ -3731,9 +3733,8 @@ ProcessResult Rugnux::RunPipeline(RugnuxObserver *observer, bool write_output, b } for (size_t i = 0; i < wanted.size(); i++) { if (const auto it = kept.find(wanted[i]); it != kept.end() && !SameSpots(it->second, found[i])) - throw JFJochException(JFJochExceptionCategory::InputParameterInvalid, - fmt::format("First-pass spot memo differs from a recomputation " - "for image {}", wanted[i])); + throw MemoMismatch(fmt::format("First-pass spot memo differs from a recomputation " + "for image {}", wanted[i])); entry->second.emplace(wanted[i], found[i]); } } @@ -3963,6 +3964,7 @@ ProcessResult Rugnux::RunPipeline(RugnuxObserver *observer, bool write_output, b const DiffractionExperiment *x = nullptr) { auto ri = std::make_unique(x ? *x : experiment_, pool); ri->MaxSpotsPerImage(first_pass_spots_per_image); + ri->VerifyMemo(config_.verify_first_pass_memo); for (size_t i = 0; i < ordinals.size(); i++) { if (cancelled_ || ri->AccumulationFull()) break; @@ -5466,10 +5468,9 @@ ProcessResult Rugnux::RunPipeline(RugnuxObserver *observer, bool write_output, b result.validation_evidence = evidence; if (memo_hit && (memo_hit->spots != evidence.spots || memo_hit->on_lattice != evidence.on_lattice || memo_hit->by_chance != evidence.by_chance)) - throw JFJochException(JFJochExceptionCategory::InputParameterInvalid, - fmt::format("First-pass memo mismatch: stored {}/{}/{}, recomputed {}/{}/{}", - memo_hit->on_lattice, memo_hit->spots, memo_hit->by_chance, - evidence.on_lattice, evidence.spots, evidence.by_chance)); + throw MemoMismatch(fmt::format("First-pass memo mismatch: stored {}/{}/{}, recomputed {}/{}/{}", + memo_hit->on_lattice, memo_hit->spots, memo_hit->by_chance, + evidence.on_lattice, evidence.spots, evidence.by_chance)); // Kept only where the pass ended on the inputs it started from, and did not take the // beam-centre ladder, which a probe (beam_center_searched_) never would. if (!geometry_prepass && !beam_center_ladder_ran && FirstPassInputKey() == first_pass_key) @@ -5657,6 +5658,7 @@ ProcessResult Rugnux::RunPipeline(RugnuxObserver *observer, bool write_output, b const CrystalLattice primary_reduced = reduced(lattice); for (int id = 1; id <= 3; id++) { RotationIndexer ri(x, *indexer_pool); + ri.VerifyMemo(config_.verify_first_pass_memo); for (const int f : spread_ordinals) { std::vector left; const auto &sp = spot_cache.at(f); diff --git a/rugnux/Rugnux.h b/rugnux/Rugnux.h index 714dba77c..23d0768b5 100644 --- a/rugnux/Rugnux.h +++ b/rugnux/Rugnux.h @@ -202,6 +202,11 @@ struct ProcessConfig { // a user-fixed -S writes nothing, because its centring absences were never integrated. bool write_p1_crosscheck = true; bool finalist_ledger = false; // --finalist-ledger; report-only symmetry evidence table + + // Where a rotation run would reuse its first-pass memos - a probe's validation evidence, a frame's + // spot list, an indexing outcome - recompute instead and throw if the two differ. Set by the tests + // that prove the reuse exact; not on the command line. + bool verify_first_pass_memo = false; }; // A rotation lattice's score on the validation frames spread over the whole sweep: their spots (off @@ -731,7 +736,7 @@ class Rugnux { // pass read (FirstPassInputKey). The first pass is deterministic in them, so an indexing probe // whose key matches takes the stored evidence instead of indexing again - the geometry walk's // probe at the geometry in hand is the canonical pass's own first pass run over. With - // RUGNUX_VERIFY_FIRST_PASS_MEMO set the probe runs anyway and the run fails if the two differ. + // ProcessConfig::verify_first_pass_memo the probe runs anyway and the run fails if the two differ. struct FirstPassMemo { std::vector key; ValidationSpotEvidence evidence; @@ -740,8 +745,8 @@ class Rugnux { // The first pass's spot lists, kept across passes under everything a frame's list depends on // (SpotFindingKey): a probe and the canonical pass after it, or a pass repeating an earlier // pass's rescue, find the spots of the same frames under the same settings again. Shared with the - // copies the run makes of itself. RUGNUX_VERIFY_FIRST_PASS_MEMO finds them again and throws on a - // difference. + // copies the run makes of itself. ProcessConfig::verify_first_pass_memo finds them again and + // throws on a difference. struct FirstPassSpots { std::mutex m; std::vector, std::map>>> entries; diff --git a/tests/RugnuxLargeTest.cpp b/tests/RugnuxLargeTest.cpp index d8b995877..19ba12005 100644 --- a/tests/RugnuxLargeTest.cpp +++ b/tests/RugnuxLargeTest.cpp @@ -79,3 +79,45 @@ TEST_CASE("Rugnux_Rotation", "[large]") { reader.Close(); REQUIRE(H5Fget_obj_count(H5F_OBJ_ALL, H5F_OBJ_ALL) == 0); } + +// A rotation run reuses what its first pass found - an indexing probe's validation evidence, each +// frame's spot list, the rotation indexer's outcome - wherever it asks the same question again: in the +// rotation-scale walk's probes and in the canonical pass after them. verify_first_pass_memo recomputes +// at every one of those hits instead and throws where the recomputation differs, so this run passing +// is the check that the reuse is exact. The geometry post-refinement and the scaling it runs inside +// are on because they are what starts the walk. +TEST_CASE("Rugnux_FirstPassMemoMatchesRecomputation", "[large]") { + const auto master = jfjoch_test::LargeDataFile("rotation_master.h5"); + if (!master) + SKIP("rotation_master.h5 not available (git-lfs data not pulled)"); + + RegisterHDF5Filter(); + JFJochHDF5Reader reader; + REQUIRE_NOTHROW(reader.ReadFile(*master)); + auto dataset = reader.GetDataset(); + REQUIRE(dataset); + + DiffractionExperiment experiment(dataset->experiment); + IndexingSettings indexing; + indexing.Algorithm(IndexingAlgorithmEnum::Auto); + indexing.RotationIndexing(true); + experiment.ImportIndexingSettings(indexing); + + ProcessConfig config; + config.mode = ProcessMode::FullAnalysis; + config.nthreads = default_threads(); + config.spot_finding = DiffractionExperiment::DefaultDataProcessingSettings(); + config.spot_finding.indexing = true; + config.rotation_indexing = true; + config.two_pass_rotation = true; + config.rotation_postrefine_geometry = true; + config.run_scaling = true; + config.verify_first_pass_memo = true; + + Rugnux process(reader, experiment, *dataset->pixel_mask, config); + ProcessResult result; + REQUIRE_NOTHROW(result = process.Run()); + CHECK(result.consensus_cell.has_value()); + + reader.Close(); +}