From e8a7696ffe88ff1c0d8244a8e14cf28c3b5484e8 Mon Sep 17 00:00:00 2001 From: Filip Leonarski Date: Thu, 30 Jul 2026 18:07:14 +0200 Subject: [PATCH] Viewer: process a dataset with the same defaults the CLI uses "Analyze dataset" and `rugnux` with no options are two front ends onto the same library and are meant to agree, but they decided their defaults separately and the two lists had drifted. The viewer was missing: - the de-novo starting point. The CLI discards the cell and space group stored in the input file before it does anything; the viewer left them on the experiment. Rugnux only searches for a space group when none is set, so the search was skipped entirely and the stored group was reported straight back. That is self-reinforcing: a finished job's own _process.h5 becomes the active snapshot, so a run that ended in P1 pinned every later run to P1 - which is what "lysozyme keeps coming out P1 in the viewer" was. - the polarization factor, so the Lp correction was omitted altogether. The missing factor is azimuthal and intensity-proportional, and symmetry mates sit at the same 2-theta but different azimuth - the exact "unequal intensities forced together" signature the space-group search vetoes as pseudo-symmetry, which can land a genuinely de-novo run in P1 on its own. - five rotation scaling defaults: the smooth-G range, the minimum captured fraction, the capture-aware sigma, outlier rejection and --search-min-zeta. The CLI's own comments tie the captured-fraction default to a crystal recovering its true space group instead of P1. Put the policy in one place (RugnuxDefaults) and have both front ends start from it. The CLI now takes its defaults from there and applies user options on top; its output is unchanged, verified bit-for-bit on four battery crystals. The stored cell/group is still available: the job dialog offers "Use the stored unit cell / space group", off by default, shown only when the file has one. Co-Authored-By: Claude Opus 5 (1M context) --- rugnux/CMakeLists.txt | 2 + rugnux/RugnuxCommandLine.cpp | 4 +- rugnux/RugnuxDefaults.cpp | 53 +++++++++++++++++++ rugnux/RugnuxDefaults.h | 31 +++++++++++ rugnux/rugnux_cli.cpp | 16 +++--- viewer/widgets/JFJochViewerSettingsDock.cpp | 7 ++- viewer/windows/JFJochProcessingJobsWindow.cpp | 45 ++++++++++++++-- viewer/windows/JFJochProcessingJobsWindow.h | 3 ++ 8 files changed, 145 insertions(+), 16 deletions(-) create mode 100644 rugnux/RugnuxDefaults.cpp create mode 100644 rugnux/RugnuxDefaults.h diff --git a/rugnux/CMakeLists.txt b/rugnux/CMakeLists.txt index c0ef52ac..a618af95 100644 --- a/rugnux/CMakeLists.txt +++ b/rugnux/CMakeLists.txt @@ -3,6 +3,8 @@ ADD_LIBRARY(Rugnux STATIC Rugnux.cpp + RugnuxDefaults.cpp + RugnuxDefaults.h Rugnux.h RugnuxCommandLine.cpp RugnuxCommandLine.h diff --git a/rugnux/RugnuxCommandLine.cpp b/rugnux/RugnuxCommandLine.cpp index 13374fa2..83118757 100644 --- a/rugnux/RugnuxCommandLine.cpp +++ b/rugnux/RugnuxCommandLine.cpp @@ -88,8 +88,8 @@ std::string RugnuxCommandLine(const ProcessConfig &config, const auto &sf = config.spot_finding; add("--spot-sigma", num(sf.signal_to_noise_threshold)); add("--spot-threshold", std::to_string(sf.photon_count_threshold)); - // The adaptive-detection default depends on the workflow (on for stills, off for rotation), so - // always emit the explicit flag rather than relying on it. + // Emit the explicit flag rather than relying on the default, so the command line reproduces + // this run even if the default changes. args.emplace_back(sf.adaptive_threshold ? "--adaptive-spots" : "--no-adaptive-spots"); if (sf.adaptive_threshold) add("--spot-false-pixels", num(sf.false_pixels_per_frame)); diff --git a/rugnux/RugnuxDefaults.cpp b/rugnux/RugnuxDefaults.cpp new file mode 100644 index 00000000..12ed7d60 --- /dev/null +++ b/rugnux/RugnuxDefaults.cpp @@ -0,0 +1,53 @@ +// SPDX-FileCopyrightText: 2026 Filip Leonarski, Paul Scherrer Institute +// SPDX-License-Identifier: GPL-3.0-only + +#include "RugnuxDefaults.h" + +namespace { + // Smoothing range for the per-frame scale G on rotation data (XDS DELPHI-like), in degrees. + constexpr double SMOOTH_G_DEG = 5.0; + // Per-observation outlier rejection for the rotation combine, in sigma. + constexpr double REJECT_OUTLIERS_NSIGMA = 6.0; +} + +void ApplyRugnuxExperimentDefaults(DiffractionExperiment &experiment) { + // The Lp correction the integrator applies to every reflection. Without it the intensities carry an + // azimuthal, intensity-proportional modulation (~1.3x at 2 A for a 1 A beam) that symmetry mates do + // NOT share, because they sit at the same 2-theta but a different azimuth - which is exactly the + // "unequal intensities forced together" signature the space-group search vetoes as pseudo-symmetry. + experiment.PolarizationFactor(0.99); +} + +void ClearStoredCrystal(DiffractionExperiment &experiment) { + experiment.SpaceGroupNumber(std::nullopt); + experiment.SetUnitCell(std::nullopt); +} + +ScalingSettings RugnuxDefaultScalingSettings(bool rotation) { + ScalingSettings s; + + // Refit the per-frame scale on the combined fulls. Rotation only - it is what lifts ISa there, and + // there is no combine on the stills path. + s.ScaleFulls(rotation); + s.SmoothGDegrees(rotation ? SMOOTH_G_DEG : 0.0); + + // Drop edge-of-sweep fulls whose rocking curve was captured less than this. 0.7 rather than 0.5 also + // strips the partiality-extrapolated fulls that dominate the intensity second moment on weakly + // diffracting crystals, without which the de-novo space-group search is starved by the error-model + // I/sigma floor and falls back to P1. + s.MinCapturedFraction(rotation ? 0.7 : 0.0); + + // Capture-aware systematic sigma: down-weights the over-extrapolated under-captured fulls. + s.CaptureUncertaintyCoeff(rotation ? 1.0 : 0.0); + + // A single un-rejected artifact wrecks the intensity-correlation and second-moment statistics the + // space-group and twinning tests are built on. + s.OutlierRejectNsigma(rotation ? REJECT_OUTLIERS_NSIGMA : 0.0); + + // Run the de-novo space-group search a second time on a merge of only the well-measured + // observations and keep whichever found more symmetry. It cannot lose symmetry, so an imperfect cut + // only means the second opinion contributes nothing. + s.SearchMinZeta(rotation ? 0.85 : 0.0); + + return s; +} diff --git a/rugnux/RugnuxDefaults.h b/rugnux/RugnuxDefaults.h new file mode 100644 index 00000000..6f42bc5e --- /dev/null +++ b/rugnux/RugnuxDefaults.h @@ -0,0 +1,31 @@ +// SPDX-FileCopyrightText: 2026 Filip Leonarski, Paul Scherrer Institute +// SPDX-License-Identifier: GPL-3.0-only + +#pragma once + +#include "../common/DiffractionExperiment.h" +#include "../common/ScalingSettings.h" + +// What "process this dataset with nothing set" means, in one place. +// +// The rugnux CLI and the viewer's "Analyze dataset" are two front ends onto the same Rugnux library, +// and they are meant to give the same answer on the same data. They used to decide these defaults +// separately, and the two lists drifted: the viewer had lost the polarization factor, five rotation +// scaling defaults, and - worst - the step that forgets the crystal the input file happens to carry, +// which silently skipped the whole de-novo space-group search and reported back whatever group was +// stored (P1, on a file written by an earlier P1 run). Both front ends now start here and then apply +// their own explicit overrides on top. + +// Analysis policy that no user option depends on. Output policy (file names, formats, compression) +// belongs to the caller - the CLI and the viewer write different things in different places. +void ApplyRugnuxExperimentDefaults(DiffractionExperiment &experiment); + +// Forget the space group and unit cell stored in the input file, so indexing and the space-group +// search start de novo. A stored cell otherwise resolves the indexing algorithm to ffbidx and drives +// it to that lattice, and a stored space group skips the search outright. A caller that has a cell or +// group from the USER (-C / -S, a reference MTZ, a dialog field) applies it after this. +void ClearStoredCrystal(DiffractionExperiment &experiment); + +// Scaling/merging defaults for the workflow. Several differ between rotation and stills because the +// rotation path has a 3D combine and the stills path does not; each is explained at its setter. +ScalingSettings RugnuxDefaultScalingSettings(bool rotation); diff --git a/rugnux/rugnux_cli.cpp b/rugnux/rugnux_cli.cpp index a6321f0e..082cce77 100644 --- a/rugnux/rugnux_cli.cpp +++ b/rugnux/rugnux_cli.cpp @@ -34,6 +34,7 @@ #include "../image_analysis/scale_merge/TwinningAnalysis.h" #include "../image_analysis/scale_merge/SearchSpaceGroup.h" #include "Rugnux.h" +#include "RugnuxDefaults.h" #include "ModelValidation.h" // Default rot3d per-frame scale-G smoothing range (XDS DELPHI-like), in degrees of rotation. @@ -447,6 +448,7 @@ std::optional parse_lattice_arg(const char *arg) { // Shared offline-output settings for the _process.h5 / reflection writer, used by both the --scale // path and the full-analysis path (each then sets its own space group and images-per-trigger). void configure_offline_output(DiffractionExperiment &experiment, const std::string &output_prefix) { + ApplyRugnuxExperimentDefaults(experiment); // analysis policy shared with the viewer experiment.BitDepthImage(32).Compression(CompressionAlgorithm::BSHUF_LZ4); // Offline CLI: the operator chose the output path, so allow an absolute -o (the multi-user guard // that FilePrefix() applies is only for remotely-supplied prefixes in the broker/writer). @@ -454,7 +456,6 @@ void configure_offline_output(DiffractionExperiment &experiment, const std::stri experiment.Mode(DetectorMode::Standard); // full image analysis experiment.PixelSigned(true); experiment.OverwriteExistingFiles(true); - experiment.PolarizationFactor(0.99); experiment.SetFileWriterFormat(FileWriterFormat::NXmxLegacy); experiment.NumTriggers(1); } @@ -1335,6 +1336,7 @@ static int RunRugnux(int argc, char **argv) { } configure_offline_output(experiment, output_prefix); + ClearStoredCrystal(experiment); // shared with the viewer; -S / -C below override it experiment.SpaceGroupNumber(space_group_number); experiment.ImagesPerTrigger(images_to_process); @@ -1408,9 +1410,9 @@ static int RunRugnux(int argc, char **argv) { // data (where it lifts ISa substantially) and off for stills. --no-scale-fulls overrides. const bool scale_fulls = scale_fulls_arg.value_or(rotation_indexing); - ScalingSettings scaling_settings; + ScalingSettings scaling_settings = RugnuxDefaultScalingSettings(rotation_indexing); scaling_settings.ScaleFulls(scale_fulls); - scaling_settings.SmoothGDegrees(smooth_g_deg_arg.value_or(rotation_indexing ? SMOOTH_G_DEFAULT_DEG : 0.0)); + scaling_settings.SmoothGDegrees(smooth_g_deg_arg.value_or(scaling_settings.GetSmoothGDegrees())); scaling_settings.RelativeBDegrees(relative_b_deg_arg.value_or(0.0)); // opt-in only; default off if (no_scaling_corrections) scaling_settings.CorrectionSurfaces(false); @@ -1430,19 +1432,19 @@ static int RunRugnux(int argc, char **argv) { // crystals, so the de-novo space-group search is no longer starved by the error-model I/sigma floor // (e.g. a weakly-diffracting F-cubic or hexagonal crystal recovers its true space group instead of // P1). An explicit --min-captured-fraction wins. - scaling_settings.MinCapturedFraction(min_captured_fraction_arg.value_or(rotation_indexing ? 0.7 : 0.0)); + scaling_settings.MinCapturedFraction(min_captured_fraction_arg.value_or(scaling_settings.GetMinCapturedFraction())); // Capture-aware systematic sigma defaults ON (1.0) for the rot3d combine - it down-weights the // over-extrapolated under-captured fulls and, with the mosaicity fix, lifts rotation ISa/anomalous // substantially. Off for non-rot3d (no combine). An explicit --capture-uncertainty always wins. - scaling_settings.CaptureUncertaintyCoeff(capture_uncertainty_arg.value_or(rotation_indexing ? 1.0 : 0.0)); + scaling_settings.CaptureUncertaintyCoeff(capture_uncertainty_arg.value_or(scaling_settings.GetCaptureUncertaintyCoeff())); scaling_settings.ForcedMosaicity(forced_mosaicity_arg); scaling_settings.MinCCForImage(min_image_cc / 100.0); // --min-image-cc is in percent; the setting is a fraction // Rotation default: run the de-novo space-group search a second time on a merge of only the // well-measured observations and keep whichever found more symmetry. It cannot lose symmetry - see // Rugnux.cpp - so the cut being imperfect only means the second opinion contributes nothing. - scaling_settings.SearchMinZeta(search_min_zeta_arg.value_or(rotation_indexing ? 0.85 : 0.0)); + scaling_settings.SearchMinZeta(search_min_zeta_arg.value_or(scaling_settings.GetSearchMinZeta())); scaling_settings.OutlierRejectNsigma( - outlier_reject_nsigma.value_or(rotation_indexing ? REJECT_OUTLIERS_DEFAULT_NSIGMA : 0.0)); + outlier_reject_nsigma.value_or(scaling_settings.GetOutlierRejectNsigma())); experiment.ImportScalingSettings(scaling_settings); diff --git a/viewer/widgets/JFJochViewerSettingsDock.cpp b/viewer/widgets/JFJochViewerSettingsDock.cpp index 25c3c385..10454670 100644 --- a/viewer/widgets/JFJochViewerSettingsDock.cpp +++ b/viewer/widgets/JFJochViewerSettingsDock.cpp @@ -229,10 +229,9 @@ QWidget *JFJochViewerSettingsDock::BuildMXPage() { auto *adaptive = new QCheckBox("Adaptive threshold (per-ring, auto)", page); adaptive->setChecked(spot_.adaptive_threshold); adaptive->setToolTip("Self-calibrating detection: the strong-pixel threshold is derived from each " - "image's own per-resolution-ring noise. The signal/noise and photon-count " - "settings are not used while this is on. Best on stills; on rotation data the " - "fixed-threshold finder is the safer choice, which is what rugnux defaults to " - "there."); + "image's own per-resolution-ring noise, so one setting adapts across datasets. " + "The signal/noise and photon-count settings are not used while this is on. " + "Default for stills and rotation alike, as in rugnux."); auto *highResSpot = new SliderPlusBox(0.5, 5.0, 0.1, 1, page); highResSpot->setValue(spot_.high_resolution_limit.value_or(1.5f)); auto *autoHighResSpot = new QCheckBox("To detector edge", page); diff --git a/viewer/windows/JFJochProcessingJobsWindow.cpp b/viewer/windows/JFJochProcessingJobsWindow.cpp index 4659d24e..68a07e6c 100644 --- a/viewer/windows/JFJochProcessingJobsWindow.cpp +++ b/viewer/windows/JFJochProcessingJobsWindow.cpp @@ -3,6 +3,7 @@ #include "JFJochProcessingJobsWindow.h" #include "JFJochMergeStatsWindow.h" +#include "../../rugnux/RugnuxDefaults.h" #include "../widgets/ToolbarIcons.h" #include "../../rugnux/RugnuxCommandLine.h" @@ -223,6 +224,20 @@ int JFJochProcessingJobsWindow::askJob(const ReprocessingInputs &inputs, JobSpec "axis from the whole sweep, then re-integrates at the refined geometry. The refined pass is the " "canonical output; the header-geometry pass is kept as _01. Default on; a no-op for stills."); + // De-novo by default, exactly as `rugnux` with no -C / -S: it discards any cell and space group + // stored in the input file so neither indexing nor the space-group search is biased by them. This is + // offered only when there IS something stored - most often because a previous run of this very + // dataset wrote it, which is how a run that once ended in P1 used to pin every later run to P1. + const bool has_stored_crystal = inputs.experiment.GetUnitCell().has_value() + || inputs.experiment.GetSpaceGroupNumber().has_value(); + auto *keep_crystal = new QCheckBox("Use the stored unit cell / space group", &dlg); + keep_crystal->setChecked(false); + keep_crystal->setEnabled(has_stored_crystal && !azint); + keep_crystal->setToolTip( + "Off (default): the cell and space group are determined from the data, discarding whatever the " + "file carries - the same as running rugnux with no -C / -S. On: index and merge with the stored " + "values instead, which is what you want for serial stills with a known cell."); + auto *form = new QFormLayout; form->addRow("Start image", start_image); form->addRow("End image", end_image); @@ -233,6 +248,8 @@ int JFJochProcessingJobsWindow::askJob(const ReprocessingInputs &inputs, JobSpec form->addRow(scaling); form->addRow(refineRow); form->addRow(postrefine); + if (has_stored_crystal && !azint) + form->addRow(keep_crystal); int result = 0; auto *run = new QPushButton("Run locally", &dlg); @@ -262,6 +279,7 @@ int JFJochProcessingJobsWindow::askJob(const ReprocessingInputs &inputs, JobSpec spec.save_h5 = save_h5->isChecked(); spec.save_merged = save_merged->isEnabled() && save_merged->isChecked(); spec.prefix = prefix->text(); + spec.keep_stored_crystal = keep_crystal->isEnabled() && keep_crystal->isChecked(); spec.scaling = scaling->isEnabled() && scaling->isChecked(); spec.refine_geometry = refine_geometry->isEnabled() && refine_geometry->isChecked(); spec.refine_geometry_frames = refine_frames->value(); @@ -315,9 +333,30 @@ void JFJochProcessingJobsWindow::newJob(bool azint) { const ProcessConfig config = buildConfig(spec, inputs); - // The experiment already carries the panel's indexing settings — including RotationIndexing set by - // "Process as stills" (needed so IndexAndRefine builds a rotation indexer) — so it is used as-is. - const DiffractionExperiment &experiment = inputs.experiment; + // The experiment carries the panel's indexing settings — including RotationIndexing set by "Process + // as stills" (needed so IndexAndRefine builds a rotation indexer). On top of that it gets exactly + // the defaults `rugnux` with no options would apply, so the two front ends agree: the analysis + // policy the panel does not expose (the polarization factor), the rotation scaling defaults, and + // the de-novo starting point. + // + // Clearing the stored crystal is the important one. Without it the space group carried in the file + // is left on the experiment, Rugnux skips the whole search (it only searches when none is set) and + // reports that group straight back — and since a finished job's own _process.h5 becomes the active + // snapshot, a run that ended in P1 pinned every later run to P1 as well. + DiffractionExperiment experiment = inputs.experiment; + ApplyRugnuxExperimentDefaults(experiment); + if (!spec.keep_stored_crystal) + ClearStoredCrystal(experiment); + if (spec.mode == ProcessMode::FullAnalysis && spec.scaling) { + ScalingSettings scaling = RugnuxDefaultScalingSettings(config.rotation_indexing); + // Keep what the settings dock does expose; take the rest from the shared defaults. + const auto &dock = inputs.experiment.GetScalingSettings(); + scaling.MergeFriedel(dock.GetMergeFriedel()); + scaling.CorrectionSurfaces(dock.GetCorrectionSurfaces()); + scaling.StillsPartialityRefine(dock.GetStillsPartialityRefine()); + scaling.HighResolutionLimit_A(dock.GetHighResolutionLimit_A()); + experiment.ImportScalingSettings(scaling); + } if (action == 2) { // copy command line const QString cmd = QString::fromStdString( diff --git a/viewer/windows/JFJochProcessingJobsWindow.h b/viewer/windows/JFJochProcessingJobsWindow.h index c042bc43..cea804cb 100644 --- a/viewer/windows/JFJochProcessingJobsWindow.h +++ b/viewer/windows/JFJochProcessingJobsWindow.h @@ -85,6 +85,9 @@ private: bool refine_geometry = false; // stills-only global geometry bundle-adjust (needs a known cell) int refine_geometry_frames = 200; // strong indexed frames fed to that bundle adjustment bool rotation_postrefine = true; // rotation-only two-pass geometry post-refine (default on) + // Process with the unit cell / space group the input file carries, instead of determining them + // de novo. Off by default, which is what `rugnux` with no -C/-S does. + bool keep_stored_crystal = false; }; // Returns 0 = cancel, 1 = run locally, 2 = copy command line; fills spec from the dialog.