v1.0.0-rc.170 #80

Merged

leonarski_f merged 17 commits from rc170 into main 2026-09-16 18:17:47 +02:00

Conversation 0 Commits 17 Files Changed 185

+1151 -469

leonarski_f commented 2026-09-16 15:24:33 +02:00

Owner

Copy Link

Copy Source

• Fixed a jfjoch_broker crash during indexing: sorting no longer misbehaves on non-finite values, and GPU FFT indexer kernel launches are now error-checked.
• rugnux needs about a third less peak memory to scale, merge and post-refine rotation data, with identical results.
• rugnux --model: the placed coordinate file carries the space group its own coordinates obey, and says so when that is not the group the reflection files beside it carry.
• jfjoch_viewer: fixes in the dataset plots, inspector and layout; spot markers lose their black outline by default (a checkbox under "Image features" restores it) and the highest-pixel markers are white boxes around the pixel.

* Fixed a `jfjoch_broker` crash during indexing: sorting no longer misbehaves on non-finite values, and GPU FFT indexer kernel launches are now error-checked. * rugnux needs about a third less peak memory to scale, merge and post-refine rotation data, with identical results. * `rugnux --model`: the placed coordinate file carries the space group its own coordinates obey, and says so when that is not the group the reflection files beside it carry. * `jfjoch_viewer`: fixes in the dataset plots, inspector and layout; spot markers lose their black outline by default (a checkbox under "Image features" restores it) and the highest-pixel markers are white boxes around the pixel.

leonarski_f added 11 commits 2026-09-16 15:24:33 +02:00

viewer: inspector and dataset-plot fixes ... f8f63fe913

- The inspector's rotation row is labelled "Goniometer angle", and its
  Smargon chi/phi are printed to one decimal, with an angle that rounds to
  zero shown as 0.0 rather than -0.
- The dataset name reserves three lines from the start, so the rows below
  it do not move as one dataset's name wraps further than the next one's.
- The stacked spots/background plots share a plot-area left edge: Qt Charts
  puts the y labels inside that margin, so a five-digit spot count used to
  start its curve further right than a one-digit background. Each compact
  plot measures its widest y label and both are padded out to the larger.
- The horizontal crosshair follows the cursor and snaps to the curve only
  within half a font height, so it can be used as a ruler anywhere on the
  plot instead of only reading back the value already in the status bar.
- Over HTTP the Smargon position never reached the experiment: the CBOR
  start message carries it, but the reader dropped it, so a live session
  showed no sample-head angles.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

v1.0.0-rc.170 ... 5e43cea90d

Version bump: VERSION, the OpenAPI spec and its three generated clients,
the docs, the frontend and the FPGA/driver version strings.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

docs: changelog entry for the viewer fixes ... f011af258f

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

viewer: label the whole x axis of a dataset plot, not the collected part ... a37f72a011

The plot's x axis already spans the whole dataset, but its goniometer-angle
ticks were computed from the images that had arrived, so a live run 10% in
labelled its first tenth at ten times the granularity and left the rest of
the axis bare. The goniometer gives the angle of an image before it is
collected, so the ticks are now laid out over the full range - still snapped
to round angles.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

docs: changelog entry for the x-axis labels ... 65dd28744c

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

viewer: wrap a long dataset name, centre the file-panel resize handle ... 0080d2f4ba

Word wrap never breaks inside a word, so a dataset name - one long token of
underscores - was laid out on a single line and cut off at the panel edge;
only a path with "/" in it wrapped at all. Break opportunities are now
marked at "/" first, at "_" next, and every twelve characters of a run with
neither, so the wrap uses the separators where they fit and breaks mid-name
only where nothing else will.

The file panel inset its contents by 4 px where every other dock sits flush,
which put its edge 4 px from the resize handle it shares with the image and
made the handle look off-centre; the handle between image and inspector was
symmetric, and now both are.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

model: the placed coordinates carry the group they obey, not the data's ... 7dc066c0f3

WritePlacedModel stamped the reflection files' space group on the placed
model unconditionally. That is right whenever the coordinates obey that
group - the ordinary case, and the subgroup case too, where the file just
holds more than one asymmetric unit of it - but wrong when the data were
merged in a supergroup of the model's group, or in the other hand. The
extra operations are then a claim the contents contradict: a refinement
program expands them, refines a structure that is not the model, and
reports nothing.

Measured on a public deposition whose data this run over-merged across a
pseudo-symmetry twofold: refining the written pair gave R-free 0.495,
where the same coordinates and the same reflections under the model's own
group give 0.285. The label alone was worth 0.21 of R-free.

The group is now taken from the argument only where every one of its
operations is one the model already has, and the model keeps its own
otherwise. The two files then disagree, which is the truth about them and
is warned about, rather than agreeing on something false.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

scale/merge: the memory peak no longer holds three copies of the partials ... bef0c6f390

The offline rotation pipeline held every partial observation in up to three
forms at once at its peak: the per-image Reflection vectors (88 B/obs),
RotationScaleMerge's flat Obs array plus its full-length GPU staging
(80 + 57 B/obs), and the geometry post-refinement's own gather (2 x 32 B/obs
while it bucket-sorts). On a rotation crystal with a ~640 A axis, 3600
finely-sliced images and 95.7 M first-pass partials that peaked at 28-30 GB
of RSS and OOMed a 32 GB machine.

Four reductions, none of which changes an output byte:

* The post-refine gather scatters each partial straight into its h bucket
  (histogram pass first), so the gathered-order copy of the whole array - one
  of two full-size copies alive through the sort - no longer exists.
* The gather is split from the fit (GatherPostRefineObservations), and the
  geometry pre-pass takes it right after Ingest, handing back each image's
  reflection vector as it is read: nothing later in the pre-pass reads them
  (its reflections are never written, and the scaling engine reads back only
  per-frame fields). A run with a reference MTZ keeps them, since resolving
  the indexing ambiguity re-reads them.
* The pre-pass frees the whole RotationScaleMerge engine - about twice the
  partial payload again - before the post-refinement fit, which sat exactly
  on the pass's peak. The canonical pass keeps everything, as before.
* Ingest's transients are bounded: the sort keys are consumed before the flat
  Obs array is built instead of coexisting with it, and the GPU upload stages
  bounded slices (under half a GB) instead of fourteen full-length arrays at
  once (5 GB on that crystal).

Measured there: peak RSS 28.3 / 30.0 GB (two baseline runs) -> 20.3-22.3 GB
(four runs of the fix), wall clock unchanged within run-to-run variance, and
byte-identical output - merged/unmerged/P1 MTZ data blocks, .hkl, .cif,
per-image table and report all equal, on this crystal and on two ordinary
rotation sentinels (which also keep their wall clock and drop slightly in
peak). What remains of the peak is the canonical pass, which cannot hand its
reflections back (they are re-seated and written); going lower means removing
the Reflection/Obs duplication itself, which this deliberately does not
attempt.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

analysis: no sort ever sees a NaN key, and FFT kernel launches are checked ... bfe95b4ede

A production broker segfaulted inside std::sort in FilterFFTResults: a NaN
length key violates strict weak ordering, and libstdc++'s unguarded partition
scan then walks off the array (confirmed from the deployed binary's faulting
instruction). No legitimate producer of that NaN exists - both FFT back-ends
emit finite lengths - so the row was corrupted, most plausibly via the one
gap in the path: neither kernel launch in FFTIndexerGPU::ExecuteFFT was
error-checked, so a failed launch silently hands back uninitialised device
memory. An audit of every sort/nth_element site then found two more places
where a NaN key is reachable by construction.

- FFTIndexerGPU: cudaGetLastError after both kernel launches (the idiom
  every other GPU translation unit already follows).
- FilterFFTResults: drop non-finite rows when building the magnitude map;
  bit-identical in normal operation.
- SearchSpaceGroup: PearsonCC deliberately returns NaN for an unscorable
  operator (n_pairs < 2, zero variance) and the score sort consumed it
  unfiltered - ~24 operators on a cubic holohedry is past the introsort
  threshold, the same crash waiting to happen. Unscorable operators now
  rank last under a well-defined comparator.
- PostIndexingRefinement: a singular QR-solved cell puts inf into
  cell.inverse() and 0*inf = NaN into the residual norms fed to
  nth_element; non-finite distances now map to +inf, which says exactly
  "this spot does not index" and orders consistently.
- ModelScaling: a NaN R factor from the first grid point latched into
  best_r and won every later comparison; it is now skipped.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

viewer: black spot outlines are opt-in, highest pixels marked by framed boxes ... 29a52ad964

The black casing rc169 put under every spot marker made the markers harder
to read for some users, so it is now off by default; the "Outline spots in
black" checkbox under "Image features" switches it on, and with it off the
spots draw exactly as before rc169. The N-highest-pixel markers change from
feature-coloured crosses to a white frame in a black casing: a frame points
at the pixel inside it, where a cross sits on top of the very pixel it
marks. The frame is slightly smaller than a spot frame and drawn above it,
so both stay visible when a top pixel falls inside a spot.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

docs: condense the rc170 changelog to one line per change ... 53a1d3d2a6

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

leonarski_f added 4 commits 2026-09-16 15:33:36 +02:00

scale/merge: hand back the host copy of the partials once the GPU holds them ... a1b63c3b5e

With the device pipeline resident and no observation dump, nothing on the host
reads the 80-byte Obs records after the upload: scaling, combine, scale-fulls
and merge run on the GPU, the ASU grouping stamps a flat group_ids array,
every pass restarts corr from corr_ingested, and the CPU combine only runs for
the dump. The one late reader, RockingEventFrames, sees only ingest-time
values on that path (the host corr is refreshed only in the dump fallback), so
Ingest takes its answer up front and frees the array - the second full-size
copy of the partial observations, held beside the source reflections for the
whole of the canonical pass. On a finely-sliced long-axis rotation crystal
with ~96 M first-pass partials that copy alone is 7.7 GB.

Byte-identical by construction: every consumer that could ever read the array
on this path is enumerated above, and the CPU and dump paths keep it exactly
as before.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

scale/merge: the resident ingest never materialises the full Obs record ... fd274d5bda

The previous commit freed the host Obs array once the device held every field;
this one stops building it. With the GPU pipeline resident (and no observation
dump) the build lays down a forty-byte record - hkl, frame, rocking position,
resolution, zeta, delta_phi, partiality, and the rocking-event usability bit -
holding exactly what the host still touches between the build and the release:
the geometry smoothing and the partiality recompute, the corr snapshot, and
the rocking-event median. Every immutable per-obs field is staged to the
device straight from the source reflections, in frame-aligned bounded slices,
and the recomputed partiality follows after the smoothing. The smoothing loops
and the rocking-event walk run on either record through the same code
(templated on the record type), so the arithmetic is exactly the one the full
record ran; the CPU and dump paths keep the full Obs as before.

On a rotation crystal with a ~640 A axis and 95.7 M first-pass partials the
full record was 7.7 GB, built and held beside the 8.4 GB of source reflections
it copied from; the narrow one is 3.8 GB and is handed back at the end of
Ingest. Measured there, on top of the previous commit's release: peak RSS
20.3-22.3 GB -> 18.3 GB, and the phases that used to carry the peak drop far
more - the second pass's post-refine probe and space-group search run at
13-15 GB where they ran at 18-20, and the first pass's post-Ingest plateau at
12-16 where it ran at 20-21. What remains on top is no longer the scale/merge:
the new peak is the first-pass ingest sitting on the image loop's own
high-water mark (~17 GB), which this code never touches.

Byte-identical output on that crystal and on two ordinary rotation sentinels:
merged, P1 and unmerged MTZ data blocks, .hkl, .cif, per-image table, and the
report modulo date/git/command line/wall time.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

docs: the rc.170 memory line reflects the further scale/merge reduction ... d752a78ee5

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Merge branch 'refobs' into rc170 ... 6dc239aaa8

# Conflicts:
#	docs/CHANGELOG.md

leonarski_f added 1 commit 2026-09-16 16:01:15 +02:00

harden the paths today's review flagged: launches checked, inputs bounded ... 968d456a11

From the five-agent review of rc168-rc170 and the ticking-bomb hunt:

- Every CUDA kernel launch is followed by cuda_err(cudaGetLastError()) (or
  the file's own check idiom) - 53 launches in 11 files ran unchecked, so a
  non-sticky launch failure (out-of-resources on a shared GPU, a zero grid)
  silently handed stale device buffers downstream as good data. The FFT
  indexer got this check in bfe95b4ed; this is the same gap everywhere else.
  BeamCenterFFTGPU already checked every launch through CheckLastKernel.
- ShadowFinder: a non-finite or absurd beam centre is refused before it can
  become a negative ring index (an out-of-bounds write) or an arbitrarily
  large per-ring table; a pixel whose polarization correction is not
  strictly positive is not usable - divided by zero it put an inf into the
  pooled means, which the running box sums turn into NaN for a whole row.
- FileWriter: the network-supplied image number is bounded by the
  collection's declared number_of_images - unbounded it sized per-image
  vectors, a huge value was a fatal allocation and a wrapping product an
  out-of-bounds heap write.
- ROICircle/ROIAzimuthal: parameters must be finite, not merely positive -
  NaN passes every <= test, inf passes > 0, and both reached the preview
  drawing where a non-finite loop bound hangs the rendering thread.
- Reader + viewer: documented that SWMR / growing HDF5 files are not
  supported - a file we open is expected to be final, which is why
  re-opening the currently open path deliberately does not re-read it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

leonarski_f added 1 commit 2026-09-16 16:13:09 +02:00

rugnux: the rescue ladder finds spots once per floor, not once per rung ... a48d26dd92

The first-pass ladder's twelve rungs are four indexer-side spot caps taken
at three resolution floors, but the loop varied the floor fastest and
apply_state clears the spot cache unconditionally, so every rung re-ran
spot finding over all sixty validation frames - and the cap is applied by
the indexer over the cached lists (MaxSpotsPerImage), so nine of those
twelve finder passes produced byte-identical lists. Group the rungs by the
finder-facing settings (rings, d_min) and re-find only when those change;
a seed change just moves the cap. Adoption is decided over all rungs at
once, so the visit order carries no meaning and the chosen rung is the
same.

On a no-crystal dataset - where every rescue fires and answers 0/60 - the
run drops from ~4 min to 1:49; datasets the ladder genuinely rescues pay
three finder passes per invocation instead of twelve.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

leonarski_f merged commit cb5a2f032a into main 2026-09-16 18:17:47 +02:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

No items

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

buntschu_d (buntschu_d) duan_j (duan_j) glettig_w (glettig_w) gotthard_g (gotthard_g) leonarski_f (leonarski_f) mx-gitea perl_d (perl_d) takaba_k (takaba_k) wang_m (wang_m)

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: mx/Jungfraujoch#80