BraggPrediction.h claimed the buffer "GROWS to whatever a frame actually
predicts, so a large cell is never truncated here". Only the two GPU Calc
overrides call GrowCapacity; both CPU predictors stop at max_reflections.
The cap is applied inside the h/k/l walk and before the resolution test,
so what survives is the low-|h| block, not the reflections nearest the
Ewald sphere - a cell large enough to overflow 20000 gives different
merged reflections with and without a GPU. Documented rather than
silently claimed otherwise.
Also removed a paragraph describing a once-per-predictor overflow warning
that no longer exists, and fixed the rugnux_cli.cpp path in HDF5.md.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The azimuthal bin limit is FPGA_INTEGRATION_BIN_COUNT = 2048, not 1024.
There are 16 ROIs, not 64, and the map is a 16-bit per-pixel mask, so a
pixel belongs to any subset of them rather than to exactly one.
The lossy transform is round(sqrt(N*N*X)) = round(N*sqrt(X)): the HLS
squares the sqrtmult register before multiplying. The doc said sqrt(N*X),
which is off by sqrt(N), and the register comment claimed the value was
"minus one" and "should be square of the coeff" - both wrong, the host
writes N and the FPGA squares it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four of the seven ceres::Solve calls in image_analysis obtained a
Solver::Summary and never looked at it, so a solve that failed numerically
had its parameters written back and was reported as success.
StillsPartialityRefine and both PostRefine solves already gated on
IsSolutionUsable(); this brings the rest to the same contract.
IsSolutionUsable() is the right test rather than checking for CONVERGENCE:
it accepts a solve that ran out of iterations or wall-clock time but still
descended, which is exactly what the real-time callers depend on when they
set max_solver_time instead of max_num_iterations. Only FAILURE and
USER_FAILURE are rejected.
XtalOptimizer checks before the write-back, so a failed refinement now
leaves the caller's geom and latt untouched instead of half-updated.
GeometryRefiner folds it into result.ok, which previously reported success
from spot and frame counts alone. RingOptimizer returns a geometry by
value that both callers assign straight back over their input, so it hands
back the unchanged reference rather than a diverged beam centre.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Verified section by section. The corrections that matter:
- image_analysis/pixel_refinement/ is gone (38ea0ec23). The style rule it
anchored - no defensive or unrequested code - stays, now attached to the
experimental analysis code generally.
- rugnux_cli.cpp lives in rugnux/, not tools/ (f737424bd).
- There is no .clang-tidy in the tree and never has been; the naming
conventions are kept, described as what the code already does.
- compression/ has no sqrt codec - the algorithms are BSHUF_LZ4 and the
three BSHUF_ZSTD variants. The square-root transform is an FPGA pipeline
stage.
- jfjoch_hdf5_test is defined in tools/, not tests/.
- JFJOCH_VIEWER_ONLY was undocumented, and is forced ON on Windows/macOS.
- The per-image-scalar recipe pointed at reader/JFJochHttpReader.cpp, which
does not exist (it is viewer/), and missed that EndMessage carries both a
per-image vector and a run-mean scalar, so the CBOR END block needs two
keys. Added the camelCase-dataset vs snake_case-field trap.
- The portability notes described work already done (libjpeg-turbo) and
recommended fetching Eigen, which CMakeLists explicitly rules out.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The queue-level fix for the live-follow OOM bounded how many datasets are
in flight, but not what each tick costs. Three handlers did full-dataset or
full-detector work per tick regardless of whether their window was open:
- the calibration window copied the whole pixel mask (GetMask returns a
reference; it was taken by value), memcpy'd it and ran a full-resolution
recolour on the GUI thread;
- the image-list window rebuilt one row of eight QStandardItems per image,
and then repainted every cell of the model on every frame to move a
one-row highlight;
- the dataset-info plot was rebuilt twice per tick, because setCurrentIndex
fires currentIndexChanged -> comboBoxSelected -> UpdatePlot and the
caller then called UpdatePlot again.
The first two now defer to showEvent while hidden, following the pattern
JFJochViewerReciprocalSpaceWindow::rebuildGL already uses; the highlight
repaints only the two rows that change; and the combo is blocked around
setCurrentIndex so the plot is built once.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
DrawSaturation walked the whole saturated set adding two QGraphicsLineItems
each, with none of the viewport culling DrawSpots and DrawPredictions do
directly above it, and no upper bound - and it is rebuilt on every pan,
zoom and frame change. Unlike spots, that set is not bounded by a setting:
an over-exposed frame or a missing beamstop saturates a large fraction of
the detector, which meant hundreds of thousands of scene items and a
multi-second freeze on each mouse drag.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The GPU merge kernel rejects outliers on the device and keeps a per-full
flag there, but only returned the per-group counts. The host array the
CPU path fills stayed all zero, and the anomalous I(+)/I(-) accumulator
is host-side and unconditional - so with --reject-outliers and a GPU
present, the observations the merged IMEAN dropped were still averaged
into I(+) and I(-). The same command on a CPU-only host excluded them:
the exported anomalous differences depended on whether a GPU was there.
R_meas was unaffected, having its own device-side path that reads the
flags in place. MergeAccum now hands the per-full flags back so every
host-side reduction sees the same rejections. The comment claiming
reject_outliers was excluded from the GPU path was never true.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was the only panel firing the generated call directly, with the
rejection routed to console.log. Because the poll then kept returning the
unchanged server value - which still equalled lastDownloadedS - the
resync effect never fired, so the dashboard showed a threshold the broker
was not using, indefinitely and silently. It now goes through useUpload
like its siblings, so a failure raises the snackbar, and onError puts the
server's value back in the panel.
The eight sliders also applied from onChange, which MUI fires for every
intermediate position while dragging: one drag across the ice-ring width
sent ~200 PUTs plus ~200 forced /statistics refetches, each reconfiguring
spot finding on the running acquisition. Dragging now only moves the
panel; the value is sent from onChangeCommitted.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
JFJochDecompressHperfPtr took source_size and never looked at it: every
per-block length was read out of the stream and passed straight to
LZ4_decompress_safe/ZSTD_decompress as the source length, with src_ptr
advanced by it. The only check happened after the whole buffer had
already been walked. A truncated frame, or a block header claiming
0x7fffffff, read far past the end of a heap buffer - reachable from the
ZeroMQ CBOR path and from any HDF5 chunk the XDS plugin is handed.
block_size == 0 satisfied the "% BSHUF_BLOCKED_MULT" test and then
divided at nelements / block_size, and source_size < 12 underflowed
source_size - 12 to about 2^64. Both are now rejected.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Deactivate() holds m for the whole power-off sequence, which is right -
nothing else should touch the detector while it is being turned off - but
it had no state check, unlike every other entry point. Called during a
measurement, calibration or initialisation it waited on measurement.get()
while holding m, and those threads re-acquire m to finish: a deadlock
that wedged every endpoint, /cancel included.
IsRunning() is exactly the set of states with a live background thread,
so deactivating stays possible from Error - otherwise a failed initialise
would leave no way to power the detector down.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The processing VDS mapping is built from the number of images a run set
out to process, while total_images comes from the end message and is the
number it actually finished. Those differ whenever a run is cancelled or
skips an unreadable frame, and the mismatch was a hard throw - which
NXmx::Finalize catches by deleting the temporary master, so rugnux lost
the entire _process.h5 and every completed image with it. Ctrl-C after
5000 of 100000 images produced no output file at all.
Map what was written and drop the remainder instead.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
rowsPerWave is rounded up, so with 32 waves the last waves can start at
or past the last row: rmin was never clamped and only the drain loop
checked front against height. On any detector below about 1500 rows -
including the module-converted 500K and 1M geometries and the kernel's
own unit tests - the priming and steady-state loops read whole rows past
the end of the image buffer, and those garbage rows entered the sliding
background window of the bottom rows.
Blocks with no rows to write now return before the first __syncthreads
(rmin depends only on blockIdx.y, so the block leaves together and the
collective ops stay well formed), and both remaining reads are bounded by
height. Rows past the end keep the INT32_MIN sentinel, which the window
already treats as "not counted".
The raw read in the steady-state loop is left as it is: making it apply
the prev_out substitution that the other two read sites use would change
which pixels are found, which is a separate question from this fix.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Character 40 carried a verbatim copy of character 35's matrix
(0-10 / -100 / 00-1), whose determinant is 1. A C-centred conventional
cell needs determinant 2, so a genuine oC lattice was returned as its
primitive monoclinic cell while still being labelled Orthorhombic 'C':
the refiner then clamped a ~117 degree beta to 90 and prediction dropped
half the reflections of a cell that has no centring.
International Tables A 3.1.3.1 gives 0-10 / 012 / -100 for character 40.
Character 35 is correct as it stands and is left alone.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
International Tables A 3.1.3.1 gives 100 / -110 / -1-13 for character 9;
the last element was -3. With a negative determinant the transform is
left-handed and the "conventional" rhombohedral cell is not hexagonal -
beta came out around 110-134 degrees instead of 90 and c was far too
long. Any R lattice tall enough to reduce to character 9 was affected,
and the downstream Trigonal->Hexagonal promotion then forced 90/90/120
onto that wrong cell.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
release() published the handle through ReleaseSlot and only then wrote
status = InPreparation. ReleaseSlot makes the handle available to
GetImageSlot immediately, and GetImageSlot hands back this very object
without resetting it, so the receiver could observe the stale Sending
status and throw "Trying to send image that is not in preparation",
aborting the collection - or take the opposite interleaving and leak the
slot.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
SendZeroCopy closes the message when zmq_msg_send fails, and closing a
message built with zmq_msg_init_data runs its free function - here
zmq_socket_free, which already calls release(). The writer thread then
released the same slot a second time, under a comment claiming the
callback would not run.
The second release put a slot back on the free list while the receiver
had already taken it for the next image, so two threads wrote the same
buffer and the sending/preparation counters drifted permanently.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The per-image ice ring score was encoded for every DataMessage but never
for the END message, although docs/CBOR.md has always listed it there and
both NXmx::EndResultVectors and HDF5MetadataSource expect it. Any dataset
written over the stream therefore had no /entry/MX/iceRingScore, and under
NXmxIntegrated - where the whole-run vector is the only copy - the score
was lost entirely.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
BraggIntegrationSettings::DMinLimit_A had a setter that nothing anywhere called, so
it was always its 1.0 A default - in rugnux, the viewer and the broker alike, with
no option or API field to change it. It feeds the predictor as high_res_A, which
discards any reflection with |q| > 1/d_min, so integration simply stopped at 1.0 A
however far the detector reached.
Five of the 33 rotation test datasets have detectors reaching past it, down to
0.981 A. On one of them, run with no resolution limit, the shell table ended dead
at 1.00 A with that shell still at CC1/2 55.6% and <I/sig> 3.4 - cut mid-shell
rather than fading out. This branch had already made the sibling limits
detector-driven (spot finding, scaling), so the pipeline was finding spots the
detector could see and then refusing to integrate them.
Make it a std::optional: unset means as far as the detector reaches, a value limits.
The limit is only a bound on how far the lattice walk goes, never a second opinion
on what is measurable - both predictors independently drop reflections that miss the
detector (BraggPrediction.cpp, BraggPredictionRot.cpp) - which is what makes the
detector's own reach the right default. rugnux gains --integration-high-resolution
(0 = no limit, as for --spot-high-resolution); the derived per-axis prediction range
resolves against the same number, so the two cannot drift.
Full battery: 30/33 space groups, unchanged from before, 0 failures and the same
three known mismatches; 22 of 32 crystals bit-identical and nothing worse than 5
observations in ~500k. The datasets that gain do so because their detector reached
past 1.0 A - the effect is understated here because the harness caps each merge at
the XDS resolution anyway.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Each Miller index is bounded by its OWN axis - |h| <= a/d_min, |k| <= b/d_min,
|l| <= c/d_min - so a single half-width has to be sized for the longest axis and
then walks the short ones far past anything the resolution cut can keep. Give the
predictor max_h, max_k and max_l instead, in all four implementations (CPU and GPU,
stills and rotation), and derive each from its own axis.
On a 149/83/226 A cell that is 23.1M candidates per frame instead of 94.2M, 4.1x
fewer. Results are bit-identical, as they must be - the candidates removed are only
ones the |q| <= 1/d_min cut rejected anyway: over six rotation crystals every merged
observation count, high-shell CC1/2 and space group matches the cube exactly, 6/6
space groups correct.
It buys almost no time, and the earlier claim that the cube cost 22% of that
crystal's wall clock was wrong. Removing 4.1x of the candidates moves it 1m58s ->
1m57s, so the whole prediction sweep is ~1% of the run. The 22% that crystal costs
relative to a fixed max_hkl of 100 is genuine extra work at max_l = 227: real
reflections inside the resolution sphere along the long axis, predicted and
integrated either way. Per-axis limits do not reduce that and cannot.
The user-facing setting stays a single number: it exists to bound the work, not to
describe the crystal, and applies to all three indices when set.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The working tree accumulates merged reflection files, models and per-image dumps
while testing, and they sit in the repository root next to the source. They are
user data: a merged .mtz/.cif carries a sample's measured unit cell and its
filename usually carries the sample's name, neither of which may enter this
repository. Only build*/ and python-client/ were ignored, so a `git add -A` would
have picked all of it up - which is exactly what happened while preparing this
branch, caught before the commit was made.
Ignore the file types rather than rely on everyone typing the right paths, and
un-ignore tests/ so checked-in fixtures still work (git add -f for anything else
that genuinely belongs). Also widen the rugnux_vs_xds.py output dir to rugnux_cmp*/,
which is where the ad-hoc comparison runs land.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Follow-up to making max_hkl a setting: it is now an optional, and unset means "take
it from this crystal". The predictor keeps only |q| <= 1/d_min and h = a.q for the
real-space axis a, so |h| <= a/d_min exactly - and likewise |k| <= b/d_min and
|l| <= c/d_min. max(a,b,c)/d_min therefore bounds all three at once: nothing that
could be predicted lies outside it, and nothing inside it is reached by a shorter
axis. It applies to rotation and stills alike, both going through the one place the
prediction settings are built.
Offline (rugnux, viewer) the default is unset, so every crystal gets its own range;
--max-hkl overrides it. Online the broker holds a concrete number, because the cost
is the cube of it per image and a live acquisition should not have its frame rate
decided by whichever sample is mounted: max_hkl joins bragg_integration_settings in
the OpenAPI with a default of 100, so an omitted field arrives as that default (the
generated model carries it) rather than as "derive it", and the frontend exposes it
next to the integration model.
Measured against a fixed 100 on six rotation crystals: three are bit-identical, two
were being truncated and recover 419k and 5.8k observations with the high-shell
CC1/2 going 15.1 -> 25.8% and 52.1 -> 55.3%, and the space group is unchanged 6/6.
It reproduces a fixed 200 exactly, which is the bound being tight rather than merely
safe.
The sixth is worth recording: a 149/83/226 A cell derives 227, and because a single
scalar has to cover the longest axis the cube is ~16x what a per-axis box would be -
22% wall clock, for a net 22 observations out of 364k (the per-frame 65536-reflection
cap re-selects at the margin when more candidates are offered) and identical CC1/2,
ISa and space group. Per-axis limits would remove that; the predictors already map a
thread index to h, k and l separately.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
max_hkl was hardcoded to 100 at the one place production builds the prediction
settings, so the only way to change it was to edit and rebuild - and it is not a
constant of the method, it is a property of the cell. An axis is truncated once
a/d_min exceeds it: 100 covers a 150 A axis at 1.5 A, but the same axis at 1.0 A,
or a 250 A axis anywhere, loses its outermost reflections with nothing said.
Move it into BraggIntegrationSettings next to the other prediction/integration
parameters and add rugnux --max-hkl (1..511, default 100 - no behaviour change).
Like the integration radii and the background trim it stays out of the OpenAPI, so
the broker keeps the default it has today and live analysis cannot be handed a
range that would not finish; the offline front end, which knows its cell, can ask
for more. RugnuxCommandLine emits it when it is not the default.
Measured on five rotation crystals at --max-hkl 200: two are bit-identical at no
cost, and three were being truncated - one gains 419k observations (+17%) and
takes its high-shell CC1/2 from 15.1% to 25.8% for +14% wall clock, the other two
gain 12k and 5.8k observations with CC1/2 76.6->82.4% and 52.1->55.3% for +9% and
+1%. ISa is unchanged throughout, and no frame overflowed the prediction buffer.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Dropping the fixed spot-finding limit left the reader still generating the
spot-vs-resolution plot over shells that stop at 1.5 A, so a stored file reopened
in the viewer showed a plot truncated at exactly the limit that was removed -
GenerateSpotPlot drops every spot outside its shells. Pass the detector's own
maximum resolution, as SpotAnalyze already does.
That value is 0 when the geometry gives no scattering angle at all (no distance or
no wavelength), and ResolutionShells throws on a non-positive d_min, once per
image. There is no resolution axis to plot against in that case, so skip the plot.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Making the worker streams non-blocking removed the implicit ordering that the
constructors were still relying on. Each engine uploads its static inputs - the
pixel mask, the pixel-to-bin map, the corrections, the ROI map - with a blocking
NULL-stream cudaMemcpy, and then reads them from kernels on its own stream. A
pageable host-to-device cudaMemcpy returns once the source has been staged, with
the DMA still in flight, and a non-blocking stream no longer waits for the NULL
stream. The failure mode is a silently unapplied mask or a stale mapping, not a
crash, so it would not have announced itself.
Put them on the stream the engine already owns, and synchronise once at the end of
the constructor - that is required for the preprocessor, whose source is a local
vector, and leaves the others settled rather than in flight for the cost of one
one-time sync. The GPU spot-finder test uploaded its image the same way.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Leaving it at G = 1 looked like the conservative choice and is the more damaging
of the two errors. The per-image scale enters as rlp/(partiality*G) and multiplies
intensity and sigma alike, so substituting 1 for a scale that was really 1/200 of
the run median puts the intensities in 200x too low with sigmas 200x too low too -
1/G^2 times the weight they deserve. The merge cannot defend itself against that,
because the number that is wrong is the number the weight is built from. And if
the collapsed value was instead a failed fit, G = 1 merges the image mis-scaled by
an unknown factor. Per-crystal scales on serial stills genuinely span orders of
magnitude, unlike frames of one rotation sweep, so both readings are live.
An image whose scale is not believable has no usable scale. Write NaN into its
image_scale_corr, which every merge path already skips on, so it drops out of the
merged intensities, the error model and the statistics consistently.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The "keep what the crystal came in with" gate required std::isfinite(cc) before it
would reject, so a refined model whose CC could not be measured at all was adopted.
ImageReferenceCC returns NaN when fewer than 20 reflections clear the partiality
cut - which is exactly what a refinement that collapsed the partialities produces,
since the cut is on the partialities it just rewrote. The gate therefore failed
open on precisely the crystals it exists to catch, and wrote the NaN into
image_scale_cc, on which --min-image-cc then drops the image from the merge, the
error model and the statistics.
Treat a CC that cannot be measured as worse than one that can, so the crystal is
put back exactly as it arrived.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
"Analyze dataset" cleared the stored cell and space group unless "Use the stored
unit cell / space group" was ticked, and that checkbox defaulted off. But the
settings panel writes the user's own cell and space group onto the experiment, so
a cell typed into the panel was discarded too - while the checkbox label said
"stored", implying it came from the file.
It also contradicted the dialog next to it: "Refine geometry (stills)" is offered
and default-ticked precisely because a cell is present, and the run then removed
that cell. The default dialog state on a stills dataset with a known cell ran the
bundle adjustment with nothing to anchor on and dropped indexing off ffbidx, which
needs a cell, onto de-novo FFT.
Drop the checkbox and take the crystal from the panel, which already has exactly
the right semantics: "Unit cell known" ticked writes the cell and group, unticked
clears both, and a space group of 0 means none. So ticked = -C/-S, unticked =
bare rugnux, and what a run will use is always what is on screen. That also keeps
the copied command line honest, since RugnuxCommandLine emits -C/-S from the same
experiment. The panel is refilled from the file when one is opened, so a finished
job's _process.h5 becoming the active snapshot now shows its group and can be
cleared, instead of silently pinning every later run to it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
--polarization was applied with the other geometry overrides, but
configure_offline_output runs afterwards and calls ApplyRugnuxExperimentDefaults,
which sets the polarization factor unconditionally. Every full-analysis run used
0.99 whatever was asked for, so the Lp correction was wrong at a beamline with
different polarization. Apply it after the defaults instead, and stop claiming in
RugnuxDefaults.h that nothing here is user-selectable.
--scale built a bare ScalingSettings and re-derived the rotation/stills split by
hand rather than calling RugnuxDefaultScalingSettings, which is what the split was
factored out for. It got scale-fulls, smooth-G, min-captured-fraction and outlier
rejection right and dropped CaptureUncertaintyCoeff on the floor: 1.0 in the
pipeline, 0.0 here. So re-scaling a rotation _process.h5 gave different sigmas and
ISa than the run that wrote it - the exact failure the block's own comment says it
exists to prevent. Start from the shared defaults and apply the overrides on top,
which also picks up --mosaicity and --search-min-zeta, and let -C bind here too.
REJECT_OUTLIERS_DEFAULT_NSIGMA had no reader left afterwards.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The kernel guards against 2*max_hkl+1 and maps thread i to h = i - max_hkl, but
the host launched a grid sized 2*max_hkl. The h = k = l = +max_hkl planes were
therefore never launched while -max_hkl was, so the GPU predicted an asymmetric
subset of what the CPU loop (inclusive on both ends) does. The same bug was fixed
on the stills twin when the whole hkl range moved to the GPU; the rotation
predictor kept the old expression.
It only bites where the cell actually reaches |h| = 100 inside d_min - a ~150 A
axis at 1.5 A - so most data never noticed. Over the 33-crystal rotation battery
29 crystals are bit-identical and 4 gain observations, all of them large-cell or
high-resolution: +8519, +4693, +901 and +758 observations, with the high-shell
CC1/2 up 15.0->15.1%, 52.0->52.2%, 76.3->76.6% and 51.6->52.1%. Nothing is lost
anywhere, and R-meas and ISa move by at most 0.01.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The first pass sampled min(n, max(refine_frames * 50, 8000)) images to keep the
200 strongest, so any serial run of 10000 frames or fewer indexed every frame
TWICE - and 99.3% of the pass was that sampling, the bundle adjust itself taking
0.24 s. The budget is sized for a low-hit-rate dataset; on data that indexes well
almost all of it was wasted.
Stop once four times the bundle size has been found, which still leaves the
"strongest N" selection a real pool and still spans the run, because the sample
is equally spaced. On a lysozyme jet dataset that is 1500 frames examined instead
of 4000, the same 200 bundled, and the same refined geometry - beam and distance
to the pixel, cell to 0.01 A. Warm cache: the pass drops 33 s -> 9.1 s and the
whole run 65 s -> 35 s, with CC1/2 and R-meas unchanged inside replicate noise.
Stills only - rotation has its own two-pass and returns from this function early.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every per-thread stream was created with cudaStreamDefault, and the 20 MB raw
image upload went to the legacy NULL stream. A NULL-stream operation implicitly
synchronises with every blocking stream in the process, so with one engine per
worker thread no two workers' GPU work could ever overlap - the whole GPU
pipeline ran serially however many threads were asked for.
Create the streams non-blocking and put the upload on the engine's own stream.
Measured on 2000 serial stills, interleaved, medians of three: 24.6 -> 19.2 s at
-N 32 (-22%), 32.7 -> 21.0 s at -N 16 (-36%), CPU utilisation 436-570% -> 723-859%.
Output bit-identical - same observations, uniques, completeness, R-meas, CC1/2,
error model and cell. The stream is synchronised at the end of the same function,
so the ordering the code relies on is unchanged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ScaleOnTheFly's collapsed-scale guard ran, and then StillsPartialityRefine
re-fitted every crystal's scale with no floor and adopted it unconditionally
whenever the image had no prior CC - which is exactly the state the guard leaves
behind. So the guard was protecting almost nothing. Measured on a lysozyme jet
dataset: of 367 images it left unscaled, only 8 were still unscaled in the
output, and 53 reached the merge at or below a fiftieth of the run median, the
worst at a 4525th; on a second run of the same sample, 297 images, worst at a
75000th. Those intensities are what the merge saw - up to 94x too high in the
written file.
Run the guard again on the refined scales. It now reports 367 then 51 on that
dataset, and the merge improves: R-meas 117.4 -> 110.8%, CC1/2 96.3 -> 96.5%.
Measurements confirm the rest of the guard is right as it stands: 0.02 is ~5x
below the lowest scale ever seen on an image that correlates with the merge
(no image at CC >= 0.4 falls below a tenth of the median), and leaving the image
at G = 1 beats both dropping it and replacing its scale with the median - on a
run where 30% of images are affected, dropping costs 1.8 CC1/2 and 29%
multiplicity.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Raising the per-image reflection limit to 65536 for offline reprocessing also
raised the image-buffer headroom derived from it, and that headroom divides a
FIXED total buffer - so every slot grew from compressed+4 MB to compressed+16.7
MB and the receiver's slot count, i.e. how much of a burst it can absorb, fell by
about three. Online never needed the raised limit: measured on three serial
stills datasets the worst frame predicts 1380 reflections, 14% of even the old
cap.
So split them, the same way the geometry refinement's stopping rule is split:
online keeps the transport-sized 10000, offline gets the full 65536, and the
buffer headroom derives from the online one. Both still come from BraggPrediction
so the cap, the prediction and the headroom cannot drift apart.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
RefineOne re-measured the image's correlation to the reference after writing the
refined partialities - because --min-image-cc drops images by it - and then
ignored what it measured. A crystal the tilt model suits worse than the fixed
partiality it replaces kept the refined model anyway, and the refinement is on by
default. Compare against the CC the crystal arrived with and put it back
untouched when the refinement does not improve it, which is the same state a
crystal with too few reflections to fit ends in.
Also four things noted in review and left until now: AdaptiveThresholdTest.cpp
was listed twice in the test target, AdaptiveThreshold.h was the one header in
image_analysis/spot_finding not in its library's source list, CLAUDE.md said
update_version.sh rewrites VERSION when it only reads it, and the CHANGELOG did
not mention that image_scale_b is gone from the plot_type enum - which breaks a
client that asks for that plot.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The per-image refinement stopped on a wall-clock budget (40 ms, and 20 ms for
the rotation-only extra pass). Online that is exactly right - the budget is real
and an image that overruns it costs the acquisition. Offline it means the same
file refines to a different lattice depending on what else the machine was doing
at the time, which is not a property reprocessing should have.
Bound it by iteration count instead when the caller is offline. IndexAndRefine
takes the workflow as a constructor argument: the receiver asks for the
wall-clock bound, rugnux and the viewer get the reproducible one. 50 iterations
is Ceres' own default; the per-image problem converges well inside it, so it
bounds the pathological case rather than the normal one - measured on five
battery crystals, every number is unchanged from the timed version.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The second pass re-indexes de novo and can land in a different setting from the
first - most often on the PRIMITIVE sub-cell of a centred lattice. The reindex
that exists to undo that declines when the metric does not match, and the code
then went on to stamp pass 1's group onto the cell regardless.
That is not a small error. A C-centred group on an already-primitive cell means
the centring absence rule removes half the reflections that genuinely exist, so
the merge holds more unique reflections than its own cell can - measured here as
"117% complete" with CC1/2 0.62, against the first pass's 92.6% and 0.98, on a
cell of exactly half the C-centred volume.
Detect the conflict where it happens and feed it to the pass-2 credibility guard
rather than acting on it locally: letting the pass re-search its own group
instead produced a P1 answer on a crystal XDS and the first pass both call C2,
which is a worse outcome than simply not trusting the pass. The completeness and
CC1/2 tests stay as the symptom-side net.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Digging into the selection logic showed the caps were not deciding the science -
the two-pass geometry post-refinement was, and the caps only fed it randomness.
Caps. The prediction buffer now grows to whatever a frame predicts instead of
keeping an arbitrary subset of it, and the per-image reflection limit is raised
to 65536, with the image-buffer transport headroom derived from the same
constant so the two cannot drift. Measured: bit-identical output on five battery
crystals, because a normal cell never approached the old limits - only a large
cell (~2.8e6 A^3, ~30000-44000 predictions per frame) ever did.
Pass-2 guard. The refined pass is normally the better answer, which is why it is
the canonical output, but it was adopted whatever it produced. On that same
crystal it merged more unique reflections than its own cell can hold -
completeness "117%", which is arithmetically impossible - while the header-
geometry pass sat at 92.6% and CC1/2 0.98. Compare the two and, when the refined
pass is not credible, go back to the header geometry and re-run so the canonical
files are the ones that are kept. Both bounds are set where only a failure
reaches them.
Together on that crystal: 111639 unique against XDS's 118730 (was 88000-99000
and different every run), CC1/2 98.0% (was 96.9-97.7%), ISa 8.54, and two runs
now agree bit for bit.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Found while chasing a 12% run-to-run spread in the merged reflection count of one
crystal. The GPU kernels claim output slots with an atomicAdd and, on overflow,
undid the increment with an atomicSub - so the counter saturated at the capacity
and the host could not tell a full buffer from an overflowing one. Which
reflections survived was then decided by CUDA block scheduling and changed every
run. Measured on that dataset: every frame predicts 23000-44000 against a 20000
buffer, and the spread reached the merged output (161591 / 165193 / 166110 /
166479 unique across four runs of the same command). Single-threaded runs diverge
too - this is entirely GPU-side.
Stop clamping the counter, so the true number predicted reaches the host, and
warn once per predictor when it exceeds the buffer. Which reflections are kept is
unchanged: making that reproducible means deciding what to keep when a frame
predicts more than the pipeline carries, and the obvious answers are worse - the
capacity is not the real limit, kPredictionOutput (10000, selected by smallest
excitation error) is, and on this crystal both a bigger buffer and a strided
selection collapse the merge, because the rotation combine rebuilds fulls from
exactly the partials that a smallest-excitation-error cut throws away.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
"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) <noreply@anthropic.com>
The error model says sigma -> b*I for strong reflections, so merged I/sigma
flattens off at 1/b - the number reported as ISa. Plotting I/sigma against I
with that asymptote drawn on it is what shows whether the reported ISa
describes the data or comes from a degenerate fit, which nothing in the window
could show before. A third page next to the per-shell plot and table.
The merge carries a few thousand strided (I, sigma) pairs to the viewer for it -
a shape, not a reflection list; the reflections themselves are in the .mtz/.cif.
The hero row gains the Wilson B and the radiation-damage Delta-B. Both were
already computed and already in MergeStatistics, so they only needed showing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The protection against a per-frame scale collapsing toward zero lived inside
ComputeSmoothGWindow, so it only existed when smooth-G did: --smooth-g=0, a
dataset whose oscillation width is unknown, and any caller that never sets a
smoothing range - the viewer among them - merged with no guard at all. A
collapsed G multiplies that frame's intensities by 1/G and its sigmas by the
same factor, so nothing downstream can see it; the merge's n-sigma cut scales
with the number that is wrong.
Pull it out into ReplaceCollapsedScales, called unconditionally right after the
partial scaling loop, and let the smooth-G window assume what it now guarantees
instead of computing its own median and floor.
The fulls guard built its median from every frame including those never fitted -
those sit at the combine's corr = 1, so a run with many unfitted frames dragged
the median toward 1 and the floor with it. It also reported the absolute
amplification where the message says "below the run median".
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Also corrects the reader bit-depth entry: taking the depth from the file was
the wrong fix for the 32-bit EIGER2 case and broke 8-bit files.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Eight pages under docs/python_client/docs describe schemas that appear nowhere
in jfjoch_api.yaml and are linked from no index - left behind because the
regeneration step never cleared python-client/, which these are copied from.
That is fixed in update_version.sh; this removes what accumulated.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
None of this has a reader:
- ScalingSettings::scaling_regularize and its setter/getter
- ScaleOnTheFlyResult::succesful (never set) and ::time_s (set, never read),
with the timing that only fed the latter
- JFJochImage::last_fit_viewport_ (written twice, read nowhere) and the
comment claiming the retry uses it - the retry keys off initial_fit_done_
- JFJochDiffractionImage::ice_ring_width_Q_recipA, and a QtConcurrent include
in a file that uses none
- an unused gemmi::Op accumulator in the spindle-angle helper
- <random> in Merge.{h,cpp}, from before the half-set split became a hash
- an orphaned comment describing the Ceres B-factor residual deleted in
014e43a4c, and two trailing comments that had collided on one line
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The script now removes the generated C++ model, the frontend client and the
published python docs before regenerating, but not python-client/ itself - and
docs/python_client/docs is filled by copying that directory. So a schema dropped
from the API kept its generated model in the PyPI package and its .md page in
the published docs, linked from no index. Eight such pages are in the tree
today, JfjochSettingsSsl among them.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adaptive spot detection became the default for rotation data as well in
6f4917dce; RUGNUX.md still said rotation kept the fixed-threshold finder, which
is also the opposite of what the usage message and CPU_DATA_ANALYSIS say.
--search-min-zeta had no entry in the option tables at all.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
centerAt returns early while the window is hidden, and nothing replays the last
position when it comes back, so re-opening the magnifier showed whatever region
the cursor was over when it was closed - with current pixels, which makes it
look like a live view of the wrong place. Remember the position while hidden and
apply it on show.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
drawPixelLabels took its range from the whole viewport rather than from the
exposed rect it was given, so a 200x40 px hover repaint still walked up to 5000
cells doing mapFromScene + QImage::pixel + drawText for each, only to have the
result clipped away. With hover feedback now rate-limited to 15 Hz that ran ~75
times a second at high zoom, against once per overlay rebuild before the
rendering rework. Intersect with the exposed rect; same in the magnifier.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The "d = ... A" readout moved from a scene item flagged
ItemIgnoresTransformations to a fixed viewport position painted in
drawForeground, which is what made a hover update dirty a small rect instead of
the whole viewport. But QGraphicsView pans by blitting the viewport: the painted
text is shifted along with the image and left there, and the pending update for
its old position is translated away too, so dragging the image smears ghost
copies of the readout across the corner. Dirty the old and the new rect when the
view scrolls - still a couple of hundred pixels, not the viewport.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
With merging on, the _process.h5 is skipped because the merged reflections are
the wanted output and that file is large (113 MB for 200 images here). But if
nothing indexes there are no merged reflections either, so the run finished
successfully having written no file at all - the one case where the user most
needs something to look at.
Write it in that case. The per-image messages have already gone past unwritten,
so this carries the dataset metadata, the mask, the azimuthal profile and the
summary scalars rather than the full per-image tables - and it is small for the
same reason it is needed (124 kB on a zero-index run). A run that does index is
untouched.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
