diff --git a/common/JFJochException.h b/common/JFJochException.h index 5a13b5bec..fa3c64809 100644 --- a/common/JFJochException.h +++ b/common/JFJochException.h @@ -44,7 +44,8 @@ CBORError, UDPError, CommunicationError, SimplonError, -CalibrationError +CalibrationError, +IndexerError }; class JFJochException : public std::exception { @@ -125,6 +126,8 @@ protected: return "Simplon API error"; case JFJochExceptionCategory::CalibrationError: return "Detector calibration error"; + case JFJochExceptionCategory::IndexerError: + return "Indexer error"; default: return ""; } diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 65d7adfd6..1625f4509 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -1,6 +1,7 @@ # Changelog ## 1.0.0 ### 1.0.0-rc.166 +* 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. * A point-group promotion that fails the merge chi-squared test needs a smaller rise in its merge's systematic error to be rescued from it, so a merohedrally twinned crystal is kept in its true lower symmetry. diff --git a/image_analysis/indexing/Indexer.h b/image_analysis/indexing/Indexer.h index dc2dc7835..26b7766e3 100644 --- a/image_analysis/indexing/Indexer.h +++ b/image_analysis/indexing/Indexer.h @@ -15,6 +15,11 @@ struct IndexerResult { std::vector lattice; float indexing_time_s; bool executed = false; // If indexing was not performed (due to indexer being not-available) mark it, so indexing result is marked accordingly + // What the indexer threw, when it threw. A THIRD outcome, not a flavour of the other two: an + // empty lattice list with no error is a real negative result about the frame - it was examined + // and no lattice fits it - whereas an error says nothing about the frame at all, the indexer + // never having got to look, and will recur on the next one. + std::optional error; }; class Indexer { diff --git a/image_analysis/indexing/IndexerThreadPool.cpp b/image_analysis/indexing/IndexerThreadPool.cpp index 30e6952ee..8684c3307 100644 --- a/image_analysis/indexing/IndexerThreadPool.cpp +++ b/image_analysis/indexing/IndexerThreadPool.cpp @@ -150,8 +150,12 @@ void IndexerThread::Worker(int threadid) { indexer.Setup(input->experiment); tmp_result = std::make_unique(indexer.Run(input->recip)); } catch (std::exception &e) { - tmp_result = nullptr; + // Hand the failure back as a result carrying the reason. A nullptr here was + // indistinguishable from a worker that was never dispatched, and both then read + // downstream as "this frame did not index". spdlog::error("Indexer thread {} failed: {}", threadid, e.what()); + tmp_result = std::make_unique(IndexerResult{ + .lattice = {}, .indexing_time_s = 0, .executed = false, .error = e.what()}); } { std::unique_lock lock(m); @@ -284,7 +288,8 @@ IndexerResult IndexerThreadPool::Run(const DiffractionExperiment &experiment, co result = tasks[task]->Run(experiment, recip); } catch (const std::exception &e) { spdlog::error("Indexer thread failed: {}", e.what()); - result = nullptr; + result = std::make_unique(IndexerResult{ + .lattice = {}, .indexing_time_s = 0, .executed = false, .error = e.what()}); } { std::unique_lock lock(m); @@ -295,5 +300,7 @@ IndexerResult IndexerThreadPool::Run(const DiffractionExperiment &experiment, co } if (result) return *result; + // No free worker, or the pool is stopping: indexing was not attempted. Distinct from both a + // frame that did not index and an indexer that failed, and left without an error for that reason. return IndexerResult{.lattice = {}, .indexing_time_s = 0}; } diff --git a/image_analysis/rotation_indexer/RotationIndexer.cpp b/image_analysis/rotation_indexer/RotationIndexer.cpp index 89b8e898a..949ce9052 100644 --- a/image_analysis/rotation_indexer/RotationIndexer.cpp +++ b/image_analysis/rotation_indexer/RotationIndexer.cpp @@ -141,6 +141,7 @@ void RotationIndexer::RunIndexing() { coords.emplace_back(rot * s.ReciprocalCoord(geom_)); } const auto indexer_result = indexer_.Run(experiment, coords); + indexer_error_ = indexer_result.error; if (!indexer_result.lattice.empty() && indexer_result.lattice[0].CalcVolume() > 1.0) { DiffractionExperiment experiment_copy(experiment); @@ -495,6 +496,11 @@ std::optional RotationIndexer::GetLattice() const { }; } +std::optional RotationIndexer::GetIndexerError() const { + std::unique_lock ul(m); + return indexer_error_; +} + void RotationIndexer::ForceResult(const RotationIndexerResult &result) { std::unique_lock ul(m); indexed_lattice = result.lattice; diff --git a/image_analysis/rotation_indexer/RotationIndexer.h b/image_analysis/rotation_indexer/RotationIndexer.h index d9b8dd51e..da24336a0 100644 --- a/image_analysis/rotation_indexer/RotationIndexer.h +++ b/image_analysis/rotation_indexer/RotationIndexer.h @@ -53,6 +53,7 @@ class RotationIndexer { size_t accumulated_spots = 0; std::optional indexed_lattice; + std::optional indexer_error_; public: // real_time: bound the candidate-cell refinements by WALL CLOCK, as online acquisition must - it // has a real budget. Offline (rugnux, the viewer) passes false and they are bounded by iteration @@ -65,6 +66,10 @@ public: std::optional angle_deg = std::nullopt); void RunIndexing(); std::optional GetLattice() const; + // What the indexer threw on the last RunIndexing(), if it threw. Empty when it simply found no + // lattice: a caller that reports the failure to the user needs the two apart, one being about + // the crystal and the other about the machine. + [[nodiscard]] std::optional GetIndexerError() const; void ForceLattice(const CrystalLattice& lattice); // Adopt a complete result (lattice + refined geometry/axis/symmetry) found by another indexer - // used when several first-pass sampling schemes are run and the best is selected. diff --git a/rugnux/Rugnux.cpp b/rugnux/Rugnux.cpp index 9b42681e4..839318b30 100644 --- a/rugnux/Rugnux.cpp +++ b/rugnux/Rugnux.cpp @@ -1798,7 +1798,10 @@ ProcessResult Rugnux::RunPipeline(RugnuxObserver *observer, bool write_output, b {"wedge", all_consecutive}, }; - struct FirstPass { std::optional result; int score = -1; std::string name; double vol = 0.0; }; + // indexer_error: what the indexer threw, if it threw. An empty result with no error is a + // verdict on the crystal; an empty result with one is a verdict on the machine. + struct FirstPass { std::optional result; int score = -1; std::string name; double vol = 0.0; + std::optional indexer_error; }; // Feed both schemes (single-threaded spot cache), run their RunIndexing() concurrently, then keep // the lattice that indexes the most validation frames on the real per-image path. @@ -1823,6 +1826,8 @@ ProcessResult Rugnux::RunPipeline(RugnuxObserver *observer, bool write_output, b for (size_t i = 0; i < ris.size(); i++) { if (cancelled_) break; + if (!bp.indexer_error) + bp.indexer_error = ris[i]->GetIndexerError(); auto found = ris[i]->GetLattice(); if (!found.has_value()) continue; @@ -2038,16 +2043,28 @@ ProcessResult Rugnux::RunPipeline(RugnuxObserver *observer, bool write_output, b release_spot_engines(); // no more frames to read; give the cards their memory back if (!cancelled_) { - // The two ways the first pass can end without a usable lattice. They are one situation from - // the user's side - no crystal was found in these frames - so both name the cause and then - // give the same remedies. + // The two ways the first pass can end without a usable lattice that are about the DATA. + // They are one situation from the user's side - no crystal was found in these frames - so + // both name the cause and then give the same remedies. constexpr const char *next_steps = " Nothing was integrated. Check the beam centre (--beam-x / --beam-y), raise " "--max-spots if the frames are well populated with spots, and use --mode azint to " "write the per-image analysis for inspection."; - if (!best.result.has_value()) + if (!best.result.has_value()) { + // An indexer that FAILED is a third outcome, and none of those remedies fit it: it + // never examined the frames, so nothing about the data or the processing options is + // implicated and changing them cannot help. Name the failure instead. + if (best.indexer_error) + throw JFJochException(JFJochExceptionCategory::IndexerError, + fmt::format("Rotation indexing could not run: {}. Nothing was " + "integrated. The indexer never examined the frames, " + "so this is a failure of the machine and not of the " + "data - free the resource it needs (a GPU shared with " + "another process is the usual cause) and run again.", + *best.indexer_error)); throw JFJochException(JFJochExceptionCategory::InputParameterInvalid, std::string("Two-pass rotation indexing found no lattice.") + next_steps); + } // A lattice that indexes almost none of the validation frames is not this crystal's lattice. // Forcing it anyway integrates every image against it; the few partials that survive yield no // fulls in the 3D combine, and the run then dies inside the merge complaining about resolution - diff --git a/tests/RotationIndexerTest.cpp b/tests/RotationIndexerTest.cpp index dec42359c..8b7b1111f 100644 --- a/tests/RotationIndexerTest.cpp +++ b/tests/RotationIndexerTest.cpp @@ -81,6 +81,9 @@ TEST_CASE("RotationIndexer") { } CHECK(cnt == 20); + // An indexer that ran records no error; only one that threw does. A caller reporting "no lattice" + // to the user tells the two apart on this. + CHECK_FALSE(indexer.GetIndexerError().has_value()); auto ret = indexer.GetLattice(); REQUIRE(ret.has_value());