diff --git a/image_analysis/IndexAndRefine.cpp b/image_analysis/IndexAndRefine.cpp index 145dad247..ef313f71f 100644 --- a/image_analysis/IndexAndRefine.cpp +++ b/image_analysis/IndexAndRefine.cpp @@ -88,6 +88,24 @@ std::optional IndexAndRefine::RotationAngle(int64_t image) const { return std::nullopt; } +void IndexAndRefine::RecordIndexerError(std::exception_ptr e) { + std::lock_guard lock(indexer_error_mutex_); + if (!first_indexer_error_) + first_indexer_error_ = e; + ++indexer_errors_; +} + +int64_t IndexAndRefine::GetIndexerErrorCount() const { + std::lock_guard lock(indexer_error_mutex_); + return indexer_errors_; +} + +void IndexAndRefine::RethrowIndexerError() const { + std::lock_guard lock(indexer_error_mutex_); + if (first_indexer_error_) + std::rethrow_exception(first_indexer_error_); +} + void IndexAndRefine::AddImageToRotationIndexer(DataMessage &msg) { if (rotation_indexer) rotation_indexer->ProcessImage(msg.number, msg.spots, RotationAngle(msg.number)); @@ -105,8 +123,13 @@ IndexAndRefine::IndexingOutcome IndexAndRefine::DetermineLatticeAndSymmetryRotat auto rot_cnt = rotation_indexer_counter.Process(msg.number); if (rot_cnt.first) rotation_indexer->ProcessImage(msg.number, msg.spots, RotationAngle(msg.number)); - if (rot_cnt.second) - rotation_indexer->RunIndexing(); + if (rot_cnt.second) { + try { + rotation_indexer->RunIndexing(); + } catch (...) { + RecordIndexerError(std::current_exception()); + } + } result = rotation_indexer->GetLattice(); } @@ -187,6 +210,8 @@ IndexAndRefine::IndexingOutcome IndexAndRefine::DetermineLatticeAndSymmetry(Data } } auto res = indexer_->Run(experiment, recip); + if (res.error) + RecordIndexerError(res.error); any_executed |= res.executed; // Kept from whichever run could measure it - the largest seed that answered - rather than from // the run whose lattice won: the severity describes the frame's lattice rows, not the cell that @@ -247,6 +272,8 @@ IndexAndRefine::IndexingOutcome IndexAndRefine::DetermineLatticeAndSymmetry(Data const auto algorithm = experiment.GetIndexingAlgorithm(); if (algorithm == IndexingAlgorithmEnum::FFT || algorithm == IndexingAlgorithmEnum::FFTW) { const auto res = indexer_->Run(experiment, recip, /*severity_only=*/true); + if (res.error) + RecordIndexerError(res.error); spindle_blind_fraction = res.spindle_blind_fraction; } if (!spindle_blind_fraction && !indexer_result.lattice.empty()) { @@ -847,7 +874,11 @@ std::optional IndexAndRefine::FinalizeRotationIndexing() if (const auto latt = rotation_indexer->GetLattice()) return latt; - rotation_indexer->RunIndexing(); + try { + rotation_indexer->RunIndexing(); + } catch (...) { + RecordIndexerError(std::current_exception()); + } return rotation_indexer->GetLattice(); } return {}; diff --git a/image_analysis/IndexAndRefine.h b/image_analysis/IndexAndRefine.h index 541603d40..e6bee48e5 100644 --- a/image_analysis/IndexAndRefine.h +++ b/image_analysis/IndexAndRefine.h @@ -5,6 +5,7 @@ #include #include +#include #include #include #include @@ -173,6 +174,13 @@ class IndexAndRefine { const IndexingOutcome &outcome, const BraggIntegrateFn &probe_integrate); + // Where the indexer failed rather than found nothing: how often, and the first failure as thrown. + // The frame is left unindexed, as one the indexer never got to; the caller decides the rest. + mutable std::mutex indexer_error_mutex_; + int64_t indexer_errors_ = 0; + std::exception_ptr first_indexer_error_; + void RecordIndexerError(std::exception_ptr e); + std::unique_ptr reindex_resolver; void ScaleImage(DataMessage &msg, IntegrationOutcome& outcome); @@ -186,6 +194,11 @@ public: bool real_time = false); void AddImageToRotationIndexer(DataMessage &msg); + // An indexer that fails says nothing about the frame and everything about the machine, so it is + // not a frame that did not index. Online, a collection goes on and reports how often it happened + // (GetIndexerErrorCount); offline, the run stops on it (RethrowIndexerError). + int64_t GetIndexerErrorCount() const; + void RethrowIndexerError() const; // the first failure, if there was one void ForceRotationIndexerLattice(const CrystalLattice& lattice); void ForceRotationIndexerResult(const RotationIndexerResult& result); // Supply a per-frame (by image number) mosaicity for prediction, overriding the per-image estimate. diff --git a/image_analysis/indexing/CUDAMemHelpers.h b/image_analysis/indexing/CUDAMemHelpers.h index 27e167491..6611beb05 100644 --- a/image_analysis/indexing/CUDAMemHelpers.h +++ b/image_analysis/indexing/CUDAMemHelpers.h @@ -5,13 +5,37 @@ #include #include +#include #include #include #include +#include #include #include "../common/JFJochException.h" #include "../common/CUDAWrapper.h" +// The message for GPU memory that could not be had: what was asked (bytes, 0 where unknown), what the +// card has free and, where it is merely full, the usual causes and the remedy - the same advice the +// scaling's own out-of-memory gives. A context lost to another fault fails here too and keeps its reason. +inline std::string GPUMemoryFailure(const char *what, size_t bytes, bool out_of_memory, const char *reason) { + char buf[512]; + if (bytes > 0) + std::snprintf(buf, sizeof buf, "%s (%.1f MB) failed (%s)", what, bytes / 1e6, reason); + else + std::snprintf(buf, sizeof buf, "%s failed (%s)", what, reason); + std::string msg(buf); + if (!out_of_memory) + return msg; + size_t free_b = 0, total_b = 0; + if (cudaMemGetInfo(&free_b, &total_b) == cudaSuccess) { + std::snprintf(buf, sizeof buf, ", with %.1f of the %.1f GB card free", free_b / 1e9, total_b / 1e9); + msg += buf; + } + return msg + ". Either another program is using this GPU, or this data set needs more GPU memory than " + "the card has - free the GPU and run again, or run on the CPU (CUDA_VISIBLE_DEVICES= rugnux " + "..., or a CPU-only build)"; +} + class CudaStream { cudaStream_t stream_ = nullptr; public: @@ -94,12 +118,16 @@ public: const int* onembed, int ostride, int odist, cufftType type, int batch) { - if (cufftPlanMany(&plan_, rank, const_cast(n), - const_cast(inembed), istride, idist, - const_cast(onembed), ostride, odist, - type, batch) != CUFFT_SUCCESS) + const cufftResult r = cufftPlanMany(&plan_, rank, const_cast(n), + const_cast(inembed), istride, idist, + const_cast(onembed), ostride, odist, + type, batch); + // Said as a memory failure whatever the code: on a full card (two runs sharing it) the plan + // failed as CUFFT_INTERNAL_ERROR, not CUFFT_ALLOC_FAILED. + if (r != CUFFT_SUCCESS) throw JFJochException(JFJochExceptionCategory::GPUCUDAError, - "Failed to create CUFFT plan with cufftPlanMany"); + GPUMemoryFailure("Creating the CUFFT plan (cufftPlanMany)", 0, true, + ("cuFFT error " + std::to_string(r)).c_str())); } // Convenience overload for vector input @@ -314,7 +342,8 @@ public: // as a card that is merely full. if (const cudaError_t err = cudaMalloc(&ptr_, count * sizeof(T)); err != cudaSuccess) throw JFJochException(JFJochExceptionCategory::GPUCUDAError, - std::string("Failed to allocate device memory (") + cudaGetErrorString(err) + ")"); + GPUMemoryFailure("Allocating GPU memory", count * sizeof(T), + err == cudaErrorMemoryAllocation, cudaGetErrorString(err))); } ~CudaDevicePtr() { release(); diff --git a/image_analysis/indexing/Indexer.h b/image_analysis/indexing/Indexer.h index cd2190145..8ed46ee6f 100644 --- a/image_analysis/indexing/Indexer.h +++ b/image_analysis/indexing/Indexer.h @@ -3,6 +3,7 @@ #pragma once +#include #include #include "../../common/CrystalLattice.h" @@ -16,11 +17,12 @@ 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; + // What the indexer threw, when it threw - the exception itself, so its category (a CUDA + // out-of-memory, say) reaches whoever decides what to do about it. 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::exception_ptr error; // How much of a single sweep's blind cone this orientation makes unrecoverable, in [0,1]. // Absent when the frame gives nothing to decide on (no spindle defined, no lattice rows). std::optional spindle_blind_fraction; diff --git a/image_analysis/indexing/IndexerThreadPool.cpp b/image_analysis/indexing/IndexerThreadPool.cpp index ea1acdc62..4afe38a97 100644 --- a/image_analysis/indexing/IndexerThreadPool.cpp +++ b/image_analysis/indexing/IndexerThreadPool.cpp @@ -155,7 +155,8 @@ void IndexerThread::Worker(int threadid) { // 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()}); + .lattice = {}, .indexing_time_s = 0, .executed = false, + .error = std::current_exception()}); // This thread goes on to the next frame, so a CUDA error left behind here would be // reported by the first cudaGetLastError() of every frame that follows - one failed // attempt would read as an indexer that never works again. @@ -295,7 +296,8 @@ IndexerResult IndexerThreadPool::Run(const DiffractionExperiment &experiment, co } catch (const std::exception &e) { spdlog::error("Indexer thread failed: {}", e.what()); result = std::make_unique(IndexerResult{ - .lattice = {}, .indexing_time_s = 0, .executed = false, .error = e.what()}); + .lattice = {}, .indexing_time_s = 0, .executed = false, + .error = std::current_exception()}); } { std::unique_lock lock(m); diff --git a/image_analysis/rotation_indexer/RotationIndexer.cpp b/image_analysis/rotation_indexer/RotationIndexer.cpp index e077d828b..dbc1e168a 100644 --- a/image_analysis/rotation_indexer/RotationIndexer.cpp +++ b/image_analysis/rotation_indexer/RotationIndexer.cpp @@ -203,7 +203,7 @@ namespace { lattice(kb, b.indexed_lattice); for (const auto &l : a.extra_lattices) lattice(ka, l); for (const auto &l : b.extra_lattices) lattice(kb, l); - return ka == kb && a.indexer_error == b.indexer_error && a.tilt_walk.has_value() == b.tilt_walk.has_value() + return ka == kb && a.tilt_walk.has_value() == b.tilt_walk.has_value() && (a.unconstrained != nullptr) == (b.unconstrained != nullptr); } } @@ -236,8 +236,7 @@ std::vector RotationIndexer::InputKey() const { } RotationIndexer::IndexingOutcome RotationIndexer::Outcome() const { - return {axis_, updated_geom_, search_result_, extra_lattices_, unconstrained_, tilt_walk_, indexed_lattice, - indexer_error_}; + return {axis_, updated_geom_, search_result_, extra_lattices_, unconstrained_, tilt_walk_, indexed_lattice}; } void RotationIndexer::RunIndexing() { @@ -263,7 +262,6 @@ void RotationIndexer::RunIndexing() { unconstrained_ = kept->unconstrained; tilt_walk_ = kept->tilt_walk; indexed_lattice = kept->indexed_lattice; - indexer_error_ = kept->indexer_error; return; } @@ -295,7 +293,10 @@ void RotationIndexer::RunIndexingLocked() { coords.emplace_back(rot * s.ReciprocalCoord(geom_)); } const auto indexer_result = indexer_.Run(experiment, coords); - indexer_error_ = indexer_result.error; + // An indexer that failed never looked at the cloud, so there is no outcome to keep, and none is + // memoised (RunIndexing): the failure goes to the caller as the indexer threw it. + if (indexer_result.error) + std::rethrow_exception(indexer_result.error); tilt_walk_.reset(); if (!indexer_result.lattice.empty() && indexer_result.lattice[0].CalcVolume() > 1.0) { @@ -891,11 +892,6 @@ 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 fe83d261c..e17e22315 100644 --- a/image_analysis/rotation_indexer/RotationIndexer.h +++ b/image_analysis/rotation_indexer/RotationIndexer.h @@ -71,7 +71,6 @@ public: std::shared_ptr unconstrained; std::optional tilt_walk; std::optional indexed_lattice; - std::optional indexer_error; }; private: mutable std::mutex m; @@ -108,7 +107,6 @@ private: size_t accumulated_spots = 0; std::optional indexed_lattice; - std::optional indexer_error_; // The accumulated per-image spots as one reciprocal-space cloud, back-rotated to phi = 0 under the // given geometry and axis. The caller holds the lock. @@ -132,12 +130,10 @@ public: // `image` is used (only valid when `image` is the goniometer's own image index). void ProcessImage(int64_t image, const std::vector& spots, std::optional angle_deg = std::nullopt); + // Throws what the indexer threw, where it failed rather than found no lattice: the two are one + // about the machine and one about the crystal, and only the second is an answer. 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/receiver/JFJochReceiver.cpp b/receiver/JFJochReceiver.cpp index 8c4aca8b3..abb36ea42 100644 --- a/receiver/JFJochReceiver.cpp +++ b/receiver/JFJochReceiver.cpp @@ -181,6 +181,17 @@ void JFJochReceiver::SendEndMessage() { rotation_indexing_lattice_type = message.rotation_lattice_type; } message.unit_cell = indexer.GetConsensusUnitCell(); + // The collection does not stop for an indexer that failed - the images are written regardless - but + // its indexing rate is then not a statement about the crystal, and the log has to say so. + if (const int64_t n = indexer.GetIndexerErrorCount(); n > 0) { + try { + indexer.RethrowIndexerError(); + } catch (const std::exception &e) { + logger.Error("Indexing failed {} times in this collection, first with: {}. Those attempts are " + "left out of the indexing rate, and a rotation lattice may be missing for the same " + "reason", n, e.what()); + } + } for (int i = 0; i < adu_histogram_module.size(); i++) message.adu_histogram["module" + std::to_string(i)] = adu_histogram_module[i]->GetHistogram(); diff --git a/rugnux/RugnuxFirstPass.cpp b/rugnux/RugnuxFirstPass.cpp index d2f814092..1b75b11c9 100644 --- a/rugnux/RugnuxFirstPass.cpp +++ b/rugnux/RugnuxFirstPass.cpp @@ -817,8 +817,6 @@ auto Rugnux::FirstPassRun::ScoreSchemes(const SchemeIndexers &ris, IndexAndRefin for (size_t i = 0; i < ris.size(); i++) { if (rx.cancelled_) break; - if (!bp.indexer_error) - bp.indexer_error = ris[i]->GetIndexerError(); auto found = ris[i]->GetLattice(); if (!found.has_value()) continue; @@ -1153,14 +1151,13 @@ auto Rugnux::FirstPassRun::FirstPassLadder(int score_before, int min_gain, int m for (size_t i = 0; i < rungs.size() && !rx.cancelled_; i++) { enter_rung(i); FirstPass alt = ScoreSchemes(rung_ris[i], *indexer); - logger.Info(" first-pass rung: {} spots/image, rings {}, stop at {} -> {}/{} frames{}", + logger.Info(" first-pass rung: {} spots/image, rings {}, stop at {} -> {}{}", rungs[i].seed == std::numeric_limits::max() ? std::string("all") : std::to_string(rungs[i].seed), rungs[i].rings ? "aside" : "kept", rungs[i].d_min ? fmt::format("{:.2f} A", *rungs[i].d_min) : std::string("the detector edge"), - alt.result.has_value() ? alt.score : -1, - static_cast(validation.size()), + alt.result.has_value() ? Frames(alt.score) + " frames" : Frames(-1), alt.result.has_value() ? fmt::format(", {}{} {:.1f} {:.1f} {:.1f} {:.1f} {:.1f} {:.1f}", alt.result->search_result.centering, @@ -1600,6 +1597,10 @@ void Rugnux::FirstPassRun::NearTie(const std::string &decision, const std::strin decision, a, b, margin, noise, model, PassTag()); } +std::string Rugnux::FirstPassRun::Frames(int score) const { + return score < 0 ? std::string("no lattice") : fmt::format("{}/{}", score, validation.size()); +} + void Rugnux::FirstPassRun::NearTieFrames(const std::string &decision, const std::string &a, int ka, const std::string &b, int kb) const { // Two lattices that both index fewer frames than the bar the run keeps a lattice on (a sixth of @@ -1611,17 +1612,16 @@ void Rugnux::FirstPassRun::NearTieFrames(const std::string &decision, const std: const double p = (std::max(k, 0) + 0.5) / (n + 1.0); return n * p * (1.0 - p); }; - NearTie(decision, fmt::format("{} {}/{}", a, ka, validation.size()), - fmt::format("{} {}/{}", b, kb, validation.size()), - ka - kb, std::sqrt(var(ka) + var(kb)), "binomial on validation frames"); + NearTie(decision, fmt::format("{} {}", a, Frames(ka)), fmt::format("{} {}", b, Frames(kb)), + std::max(ka, 0) - std::max(kb, 0), std::sqrt(var(ka) + var(kb)), "binomial on validation frames"); } void Rugnux::FirstPassRun::NearTieGate(const std::string &decision, const std::string &what, int score, double gate) const { const double n = static_cast(validation.size()); const double p = (std::max(score, 0) + 0.5) / (n + 1.0); - NearTie(decision, fmt::format("{} {}/{}", what, score, validation.size()), - fmt::format("gate {:.1f}", gate), score - gate, std::sqrt(n * p * (1.0 - p)), + NearTie(decision, fmt::format("{} {}", what, Frames(score)), + fmt::format("gate {:.1f}", gate), std::max(score, 0) - gate, std::sqrt(n * p * (1.0 - p)), "binomial on validation frames"); } diff --git a/rugnux/RugnuxFirstPass.h b/rugnux/RugnuxFirstPass.h index 7ca66ce90..b91870ff0 100644 --- a/rugnux/RugnuxFirstPass.h +++ b/rugnux/RugnuxFirstPass.h @@ -28,13 +28,12 @@ #include "../image_analysis/MXAnalysisWithoutFPGA.h" namespace rugnux_internal { - // A candidate lattice of the first pass and what it scored on the validation frames. - // 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. + // A candidate lattice of the first pass and what it scored on the validation frames; no result + // (score -1) where nothing indexed. An indexer that failed is not that: it throws (RunIndexing). // larger_settled: the result is the larger cell of an axis-harmonic pair, and the arbiter in // the scheme comparison established it on the spots. struct FirstPass { std::optional result; int score = -1; std::string name; double vol = 0.0; - std::optional indexer_error; bool larger_settled = false; }; + bool larger_settled = false; }; // Two results are one lattice when their Niggli-reduced primitive edges agree within 2 % (the // battery's lattice-identity test), whatever setting or class each is held in. @@ -151,6 +150,8 @@ struct Rugnux::FirstPassRun { // and for two pooled spot shares the binomial standard error ValidationEvidencePrefers uses. static constexpr double NEAR_TIE_SIGMA = 2.0; std::string PassTag() const; + // A frame count as the log gives it: "k/N", or "no lattice" for a pass that found none. + std::string Frames(int score) const; void NearTie(const std::string &decision, const std::string &a, const std::string &b, double margin, double noise, const char *model) const; // Two frame counts, a against b; nothing where neither reaches the run's floor of a sixth. diff --git a/rugnux/RugnuxFirstPassAccept.cpp b/rugnux/RugnuxFirstPassAccept.cpp index 55fd883f5..1df384d0b 100644 --- a/rugnux/RugnuxFirstPassAccept.cpp +++ b/rugnux/RugnuxFirstPassAccept.cpp @@ -103,20 +103,8 @@ namespace { } void Rugnux::FirstPassRun::RequireLattice(const FirstPass &best) const { - 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)); + if (!best.result.has_value()) throw NoLatticeFound(std::string("Two-pass rotation indexing found no lattice.") + next_steps); - } } // Whether the run has a lattice at all is decided here, on the pooled spots rather than on diff --git a/rugnux/RugnuxFirstPassRescues.cpp b/rugnux/RugnuxFirstPassRescues.cpp index 4863f6116..3c1d8f8cc 100644 --- a/rugnux/RugnuxFirstPassRescues.cpp +++ b/rugnux/RugnuxFirstPassRescues.cpp @@ -130,21 +130,21 @@ auto Rugnux::FirstPassRun::AxisSignRescue(FirstPass best) -> FirstPass { // the flipped lattice must also beat chance on the pooled spots - the test the run puts // its final lattice to - which a sparse pattern at the right sign passes on few frames // (2/60 frames, 89% of the validation spots against 0% by chance). - const bool alt_real = alt.result.has_value() && alt.score > best.score - && ValidationEvidenceBeatsChance(MeasurePooled(*indexer, *alt.result)); + // A file's sign that found no lattice counts as 0 frames: no answer is not a count the + // opposite sign can beat with another 0. + const bool more_frames = alt.result.has_value() && alt.score > std::max(best.score, 0); + const bool alt_real = more_frames && ValidationEvidenceBeatsChance(MeasurePooled(*indexer, *alt.result)); if (alt_real) { - logger.Info("Rotation axis sign: the file's axis indexes {}/{} validation frames, " - "the opposite sign {}/{} - adopting the opposite sign for this run", - best.score, static_cast(validation.size()), - alt.score, static_cast(validation.size())); + logger.Info("Rotation axis sign: validation frames indexed at the file's axis {}, at the " + "opposite sign {} - adopting the opposite sign for this run", + Frames(best.score), Frames(alt.score)); best = alt; } else { - if (alt.result.has_value() && alt.score > best.score) - logger.Info("Rotation axis sign: the opposite sign indexes {}/{} validation frames " - "against the file's {}/{}, but its lattice does not beat chance on the " - "pooled spots - the file's sign is kept", - alt.score, static_cast(validation.size()), - best.score, static_cast(validation.size())); + if (more_frames) + logger.Info("Rotation axis sign: validation frames indexed at the opposite sign {}, at " + "the file's axis {}, but its lattice does not beat chance on the pooled " + "spots - the file's sign is kept", + Frames(alt.score), Frames(best.score)); rx.experiment_.Goniometer(*gon); // the file was right; put it back } } @@ -221,7 +221,7 @@ auto Rugnux::FirstPassRun::LongAxisRescue(const FirstPass &standing) -> std::opt rx.experiment_.SetUnitCell(std::nullopt); // leave the space-group / cell determination de-novo if (constrained.result.has_value()) NearTieFrames("long-axis rescue", "re-index", constrained.score, "standing", standing.score); - if (constrained.result.has_value() && constrained.score > standing.score) + if (constrained.result.has_value() && constrained.score > std::max(standing.score, 0)) return constrained; } return std::nullopt; @@ -296,10 +296,8 @@ auto Rugnux::FirstPassRun::BeamCentreCheck(FirstPass best, bool &long_axis_asked if (measured_indexes) return CentreWhenBothIndex(std::move(best), alt, c); logger.Info("Beam centre check: the measured centre ({:.2f},{:.2f}) indexes " - "{}/{} validation frames against the file's {}/{} - the file's " - "centre stands", measured_x, measured_y, alt.score, - static_cast(validation.size()), best.score, - static_cast(validation.size())); + "{} validation frames against the file's {} - the file's " + "centre stands", measured_x, measured_y, Frames(alt.score), Frames(best.score)); return best; } @@ -355,15 +353,13 @@ auto Rugnux::FirstPassRun::CentreWhenFileFails(FirstPass best, FirstPass alt, co const FirstPass both = PickBest(*indexer_pool, *indexer); if (both.result.has_value() && both.score > majority) { logger.Warning("Beam centre from the background: the file's centre " - "({:.2f},{:.2f}) indexes {}/{} validation frames and the " - "measured ({:.2f},{:.2f}) {}/{}, but the measured centre " - "WITH the opposite rotation-axis sign indexes {}/{}. The " + "({:.2f},{:.2f}) indexes {} validation frames and the " + "measured ({:.2f},{:.2f}) {}, but the measured centre " + "WITH the opposite rotation-axis sign indexes {}. The " "file has both wrong, and neither shows while the other " "stands; the run takes the measured centre and the opposite " - "sign", header_x, header_y, best.score, - static_cast(validation.size()), measured_x, measured_y, - alt.score, static_cast(validation.size()), both.score, - static_cast(validation.size())); + "sign", header_x, header_y, Frames(best.score), measured_x, measured_y, + Frames(alt.score), Frames(both.score)); alt = both; measured_indexes = true; joint = true; @@ -427,21 +423,20 @@ auto Rugnux::FirstPassRun::CentreWhenFileFails(FirstPass best, FirstPass alt, co rx.beam_center_alternative_ = std::array{measured_x, measured_y}; rx.beam_center_alternative_state_ = away_state; logger.Warning("Beam centre check: reading less of each frame, the " - "file's centre indexes {}/{} validation frames and the " - "measured centre {:.2f} px away {}/{}, on the same " + "file's centre indexes {} validation frames and the " + "measured centre {:.2f} px away {}, on the same " "lattice - the counts cannot say which centre is right, " "so the run carries BOTH centres forward and adopts " "whichever measures more of the crystal", - home->score, static_cast(validation.size()), moved, - away->score, static_cast(validation.size())); + Frames(home->score), moved, + Frames(away->score)); } else logger.Warning("Beam centre check: reading less of each frame takes the " - "file's centre to {}/{} validation frames against {}/{} at " + "file's centre to {} validation frames against {} at " "the measured centre {:.2f} px away - it is how deep this " "run was reading and not the centre, so the centre stays", - home->score, static_cast(validation.size()), - away ? away->score : 0, - static_cast(validation.size()), moved); + Frames(home->score), + Frames(away ? away->score : 0), moved); } else if (away) { alt = *away; measured_indexes = true; @@ -452,20 +447,17 @@ auto Rugnux::FirstPassRun::CentreWhenFileFails(FirstPass best, FirstPass alt, co if (lean) logger.Warning("Beam centre from the background: neither centre indexes the " "whole spot list, but at the measured ({:.2f},{:.2f}) the " - "lean first pass above indexes {}/{} against the file's " - "{}/{} - the beam centre in this file is wrong, and only a " + "lean first pass above indexes {} against the file's " + "{} - the beam centre in this file is wrong, and only a " "pass that reads less of each frame could see it", - measured_x, measured_y, alt.score, - static_cast(validation.size()), best.score, - static_cast(validation.size())); + measured_x, measured_y, Frames(alt.score), Frames(best.score)); else if (!joint) logger.Warning("Beam centre from the background: the file's ({:.2f},{:.2f}) " - "indexes {}/{} validation frames and the measured " - "({:.2f},{:.2f}) indexes {}/{} - the beam centre in this " + "indexes {} validation frames and the measured " + "({:.2f},{:.2f}) indexes {} - the beam centre in this " "file is wrong, and the run adopts the measured one", - header_x, header_y, best.score, - static_cast(validation.size()), measured_x, measured_y, - alt.score, static_cast(validation.size())); + header_x, header_y, Frames(best.score), measured_x, measured_y, + Frames(alt.score)); best = alt; } else if (!deeper_at_home) { best = CentreByPooledSpots(std::move(best), std::move(alt), c, long_axis_asked); @@ -513,13 +505,13 @@ auto Rugnux::FirstPassRun::CentreByPooledSpots(FirstPass best, FirstPass alt, co && ValidationEvidencePrefers(at_header, at_measured)) { TryBeamCenter(measured_x, measured_y); logger.Warning("Beam centre from the background: neither centre indexes a " - "majority of the validation frames ({}/{} at the measured, " - "{}/{} at the file's), but the pooled spots put " + "majority of the validation frames ({} at the measured, " + "{} at the file's), but the pooled spots put " "{:.1f}% on the lattice at the measured ({:.2f},{:.2f}) against " "{:.1f}% at the file's (chance {:.1f}% and {:.1f}%) - the run " "adopts the measured centre", - alt.score, static_cast(validation.size()), - best.score, static_cast(validation.size()), + Frames(alt.score), + Frames(best.score), 100.0 * at_measured.on_lattice / std::max(1, at_measured.spots), measured_x, measured_y, 100.0 * at_header.on_lattice / std::max(1, at_header.spots), @@ -527,12 +519,11 @@ auto Rugnux::FirstPassRun::CentreByPooledSpots(FirstPass best, FirstPass alt, co 100.0 * at_header.by_chance / std::max(1, at_header.spots)); best = alt; } else { - logger.Info("Beam centre check: the file's centre ({}/{} frames) and the " - "measured one {:.2f} px away ({}/{}) - the measured centre puts no " + logger.Info("Beam centre check: the file's centre ({} frames) and the " + "measured one {:.2f} px away ({}) - the measured centre puts no " "more of the pooled spots on its lattice, so the file's centre is " "kept", - best.score, static_cast(validation.size()), moved, alt.score, - static_cast(validation.size())); + Frames(best.score), moved, Frames(alt.score)); } return best; } @@ -643,17 +634,17 @@ auto Rugnux::FirstPassRun::CentresDisagree(FirstPass best, const FirstPass &alt, logger.Warning("Beam centre check: the two centres DISAGREE about the " "lattice. The file's ({:.2f},{:.2f}) gives {}-centred {}, " "{:.2f} {:.2f} {:.2f} {:.2f} {:.2f} {:.2f} (primitive volume " - "{:.0f} A^3, {}/{} frames); the measured ({:.2f},{:.2f}) gives " + "{:.0f} A^3, {} frames); the measured ({:.2f},{:.2f}) gives " "{}-centred {}, {:.2f} {:.2f} {:.2f} {:.2f} {:.2f} {:.2f} " - "(primitive volume {:.0f} A^3, {}/{} frames){}", + "(primitive volume {:.0f} A^3, {} frames){}", header_x, header_y, best.result->search_result.centering, gemmi::crystal_system_str(best.result->search_result.system), hc.a, hc.b, hc.c, hc.alpha, hc.beta, hc.gamma, best.vol, - best.score, static_cast(validation.size()), + Frames(best.score), measured_x, measured_y, alt.result->search_result.centering, gemmi::crystal_system_str(alt.result->search_result.system), mc.a, mc.b, mc.c, mc.alpha, mc.beta, mc.gamma, alt.vol, - alt.score, static_cast(validation.size()), + Frames(alt.score), harmonic ? fmt::format(" - the volumes differ by {:.2f}x, so one " "is an axis harmonic of the other and the " "beam centre is what settles which", @@ -965,9 +956,9 @@ auto Rugnux::FirstPassRun::SubLatticeHypothesis(FirstPass best) -> FirstPass { at_standing.on_lattice, at_standing.spots, at_standing.by_chance, adopt ? "adopting it" : "keeping the standing cell"); } - logger.Info("Sub-lattice hypothesis: the standing cell ({:.0f} A^3) indexes {}/{} validation " + logger.Info("Sub-lattice hypothesis: the standing cell ({:.0f} A^3) indexes {} validation " "frames; its index-2 sub-lattices (half-translation u,v,w):{}{}", - best.vol, best.score, static_cast(validation.size()), counts, verdict); + best.vol, Frames(best.score), counts, verdict); if (adopt) best = std::move(*sub_best); } diff --git a/rugnux/RugnuxImagePass.cpp b/rugnux/RugnuxImagePass.cpp index 05e24de27..caa246819 100644 --- a/rugnux/RugnuxImagePass.cpp +++ b/rugnux/RugnuxImagePass.cpp @@ -269,6 +269,9 @@ void Rugnux::ImagePass(PipelineLocals &p) { logger.Error("Error analyzing image {}: {}", image_idx, e.what()); continue; } + // An indexer that failed left this frame unindexed without looking at it; the run stops + // rather than merge what the machine dropped. + indexer->RethrowIndexerError(); pinned.reset(); msg.processing_time_s = std::chrono::duration(std::chrono::steady_clock::now() - t0).count(); msg.run_number = experiment_.GetRunNumber(); @@ -571,7 +574,9 @@ void Rugnux::ImagePass(PipelineLocals &p) { end_msg.indexing_rate = result.indexing_rate; if (full && !cancelled_) { - if (const auto rot = indexer->FinalizeRotationIndexing(); rot.has_value()) { + const auto rot = indexer->FinalizeRotationIndexing(); + indexer->RethrowIndexerError(); + if (rot.has_value()) { last_rotation_result_ = *rot; end_msg.rotation_lattice = rot->lattice; // The refined geometry, not the nominal StartMessage values. Two things about these five diff --git a/rugnux/RugnuxPasses.cpp b/rugnux/RugnuxPasses.cpp index 16836812b..4c10582ec 100644 --- a/rugnux/RugnuxPasses.cpp +++ b/rugnux/RugnuxPasses.cpp @@ -423,6 +423,14 @@ ProcessResult Rugnux::RunAllPasses(RugnuxObserver *observer) { const auto file_mosaicity = prepass_mosaicity_; const auto file_geometry = prepass_detector_geometry_; const auto file_result = prepass_result_; + // What else a pass leaves on the run that a later pass reads. The arm's own reflection + // footprint replaced the canonical integration settings and its last rotation result the + // doubled-cell pass's starting point, so an arm that lost still moved the result. + const auto file_bragg_adaptive = bragg_adaptive_; + const auto file_bragg_before_adaptive = bragg_before_adaptive_; + const auto file_bkg_starved_fraction = bkg_starved_fraction_; + const auto file_last_rotation_result = last_rotation_result_; + const bool file_supercell_settled = supercell_settled_; experiment_ = experiment_before_first_pass; // The measured spot width is a property of the crystal and was measured once, before this @@ -441,7 +449,9 @@ ProcessResult Rugnux::RunAllPasses(RugnuxObserver *observer) { } catch (const std::exception &e) { // A hypothesis that does not survive its own pass is not a failure of the run: the // file's centre has already produced a complete first pass, and this arm exists only - // to be compared against it. + // to be compared against it. A machine that ran out of a resource is, though: the + // file's centre would meet it next. + if (IsFatalResourceError(e)) throw; logger.Warning("Beam centre check: the pass at the measured centre did not complete " "({}) - the run keeps the file's centre", e.what()); } @@ -502,6 +512,11 @@ ProcessResult Rugnux::RunAllPasses(RugnuxObserver *observer) { prepass_mosaicity_ = file_mosaicity; prepass_detector_geometry_ = file_geometry; prepass_result_ = file_result; + bragg_adaptive_ = file_bragg_adaptive; + bragg_before_adaptive_ = file_bragg_before_adaptive; + bkg_starved_fraction_ = file_bkg_starved_fraction; + last_rotation_result_ = file_last_rotation_result; + supercell_settled_ = file_supercell_settled; } // The arm's own pass asks the same question again at its own centre; the run has already // decided it, so nothing is to consume a second answer. diff --git a/rugnux/RugnuxScaleMerge.cpp b/rugnux/RugnuxScaleMerge.cpp index 1b6dd982f..6fd346d02 100644 --- a/rugnux/RugnuxScaleMerge.cpp +++ b/rugnux/RugnuxScaleMerge.cpp @@ -749,7 +749,9 @@ bool Rugnux::ScaleMergeAndSymmetry(PipelineLocals &p) { } } if (result.consensus_cell.has_value()) { - if (const auto rot = indexer->FinalizeRotationIndexing(); rot && rot->axis) { + const auto rot = indexer->FinalizeRotationIndexing(); + indexer->RethrowIndexerError(); + if (rot && rot->axis) { if (geometry_prepass) prepass_result_ = *rot; // full result (lattice+geom+axis), for the supercell fallback PostRefineSettings prs; diff --git a/rugnux/RugnuxStills.cpp b/rugnux/RugnuxStills.cpp index e2e6ab916..2c938876d 100644 --- a/rugnux/RugnuxStills.cpp +++ b/rugnux/RugnuxStills.cpp @@ -191,6 +191,7 @@ void Rugnux::RefineStillsGeometry(int start_image, int end_image, int images_to_ if (IsFatalResourceError(e)) throw; continue; } + indexer.RethrowIndexerError(); // a frame the indexer failed on is not one that did not index if (!msg.indexing_result.value_or(false) || !msg.indexing_lattice.has_value()) continue; diff --git a/tests/CudaHandledErrorTest.cpp b/tests/CudaHandledErrorTest.cpp index 9889ddfc1..101dd9f0d 100644 --- a/tests/CudaHandledErrorTest.cpp +++ b/tests/CudaHandledErrorTest.cpp @@ -29,7 +29,9 @@ TEST_CASE("CudaDevicePtr_FailedAllocationLeavesErrorUntilCleared", "[CUDAMemHelp SKIP("No CUDA GPU present"); cuda_clear_error(); - REQUIRE_THROWS(CudaDevicePtr(std::numeric_limits::max(), CudaAlloc::Synchronous)); + // Said as what it is, with the remedy, not as a bare CUDA status. + REQUIRE_THROWS_WITH(CudaDevicePtr(std::numeric_limits::max(), CudaAlloc::Synchronous), + Catch::Matchers::ContainsSubstring("another program is using this GPU")); // Work that goes fine in between changes nothing: the error is still waiting. CudaDevicePtr fine(1 << 16); diff --git a/tests/RotationIndexerTest.cpp b/tests/RotationIndexerTest.cpp index a98f74fe6..5bf029ed6 100644 --- a/tests/RotationIndexerTest.cpp +++ b/tests/RotationIndexerTest.cpp @@ -75,9 +75,6 @@ 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());