v1.0.0.rc-162 #72
Merged
leonarski_f
merged 108 commits from 2026-08-25 08:21:40 +02:00
2608-rc-162 into main
108
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
3ed7628dd5 |
Search the primitive cell when re-seating a lattice into a fixed group's setting
Build Packages / build:windows:nocuda (push) Successful in 17m13s
Build Packages / build:windows:cuda (push) Successful in 18m54s
Build Packages / build:viewer-tgz:cpu (push) Successful in 21m26s
Build Packages / build:viewer-tgz:cuda (push) Successful in 22m38s
Build Packages / build:rpm (ubuntu2404_nocuda) (push) Successful in 22m42s
Build Packages / build:rpm (rocky9_nocuda) (push) Successful in 23m54s
Build Packages / build:rpm (rocky8_nocuda) (push) Successful in 28m16s
Build Packages / build:rpm (ubuntu2204_nocuda) (push) Successful in 29m21s
Build Packages / build:rpm (rocky8_sls9) (push) Successful in 29m29s
Build Packages / build:windows:nocuda (pull_request) Successful in 12m58s
Build Packages / build:windows:cuda (pull_request) Successful in 21m15s
Build Packages / XDS test (durin plugin) (push) Successful in 11m53s
Build Packages / build:rpm (rocky9_sls9) (push) Successful in 21m26s
Build Packages / build:rpm (rocky9) (push) Successful in 23m9s
Build Packages / Generate python client (push) Successful in 52s
Build Packages / Build documentation (push) Successful in 1m35s
Build Packages / Create release (push) Skipped
Build Packages / build:rpm (ubuntu2404) (push) Successful in 22m17s
Build Packages / build:rpm (ubuntu2204) (push) Successful in 27m18s
Build Packages / build:rpm (rocky8) (push) Successful in 28m44s
Build Packages / XDS test (JFJoch plugin) (push) Successful in 11m13s
Build Packages / XDS test (neggia plugin) (push) Successful in 9m58s
Build Packages / DIALS test (push) Successful in 24m5s
Build Packages / build:viewer-tgz:cpu (pull_request) Successful in 15m14s
Build Packages / build:viewer-tgz:cuda (pull_request) Successful in 17m16s
Build Packages / build:rpm (rocky9_nocuda) (pull_request) Successful in 21m8s
Build Packages / build:rpm (ubuntu2404_nocuda) (pull_request) Successful in 21m13s
Build Packages / build:rpm (rocky8_nocuda) (pull_request) Successful in 23m51s
Build Packages / build:rpm (ubuntu2204_nocuda) (pull_request) Successful in 23m49s
Build Packages / build:rpm (rocky8_sls9) (pull_request) Successful in 20m44s
Build Packages / build:rpm (rocky9_sls9) (pull_request) Successful in 19m15s
Build Packages / build:rpm (rocky9) (pull_request) Successful in 19m58s
Build Packages / build:rpm (ubuntu2404) (pull_request) Successful in 22m1s
Build Packages / XDS test (durin plugin) (pull_request) Successful in 11m7s
Build Packages / Generate python client (pull_request) Successful in 35s
Build Packages / build:rpm (ubuntu2204) (pull_request) Successful in 25m21s
Build Packages / Build documentation (pull_request) Successful in 1m2s
Build Packages / Create release (pull_request) Skipped
Build Packages / build:rpm (rocky8) (pull_request) Successful in 26m44s
Build Packages / XDS test (JFJoch plugin) (pull_request) Successful in 9m16s
Build Packages / XDS test (neggia plugin) (pull_request) Successful in 7m46s
Build Packages / DIALS test (pull_request) Successful in 20m43s
Build Packages / Unit tests (push) Successful in 2h28m34s
Build Packages / Unit tests (pull_request) Successful in 2h39m23s
rotation_lattice is the CONVENTIONAL cell and carries its centring beside it, but LatticeSearchForClass reaches the character table through a Niggli reduction that takes whatever it is handed to be primitive. This arm is entered exactly when the indexed centring differs from the fixed group's, which includes every centred indexed lattice - so the cell was being reduced as a lattice with the wrong point set. The other lattice arithmetic in this file already goes through ToPrimitive for the same reason. What that could cost: a C-centred orthorhombic conventional cell read as primitive is itself a perfectly good oP metric, so -S P212121 on one would match character 32, reindex by the identity with a residual of zero, and report that it had re-seated the reflections. The centring then agrees, so the guard added alongside this arm - which exists to stop exactly that merge - never fires, and the run merges with an absence rule that deletes half the reflections the crystal has. Measured on the 38-crystal rotation battery, every crystal pinned to its XDS reference space group with the new --force-xds-sg: 35/38 groups correct, and the same 3 runs abort before and after. The fixed-group arm is reached by 4 crystals; 3 of them index P-centred, where ToPrimitive is the identity. The one crystal that feeds a genuinely centred cell in (F-centred cubic against a reference P3121) refuses either way, with a byte-identical message. Both crystals the arm re-seats successfully (P -> C 1 2 1) produce byte-identical merged CIFs. So this removes the hazard without moving a result. --force-xds-sg is new: the harness only ever ran de novo, so the fixed-group path had no coverage in it at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y5XisyYxmF8mUEQjzpMRe2 |
||
|
|
4870c0214a |
OpenAPI: default the file writer format to NXmxVDS, and regenerate
The schema declared NXmxLegacy while the server's own default has long been
NXmxVDS (FileWriterSettings.h), and the integration page added this release
tells users NXmxVDS is the one to use - legacy is the layout DIALS reads only
the first data file of. A generated client that fills in schema defaults was
therefore quietly asking for the layout we advise against.
The C++ server does not materialise the default at all - an absent enum reaches
Convert as INVALID_VALUE and throws - so this moves nothing server-side. It
moves the python and zod clients, which do fill defaults in, and the changelog
says so.
update_version.sh also picked up a grid-scan description that had been changed
on this branch but never regenerated into the clients ("mutually exclusive with
rotation_axis", which stopped being true when the two stopped being
alternatives).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y5XisyYxmF8mUEQjzpMRe2
|
||
|
|
00535a7486 |
Give a grid scan's stationary spindle one entry per image
A grid scan with no goniometer and no Smargon head has the placeholder omega as the only rotation axis in its sample chain - grid_scan_x and grid_scan_y are translations, and a reader looking for a scan axis passes over them. Written as a scalar, the whole scan therefore read back as a single image. This is the defect the Smargon change fixed for a still at a head position, and its reasoning applies here verbatim: a reader takes the image count from the innermost axis of the chain when no axis varies. Write the placeholder the same way, one entry per image. The reader round-trip could not see it - our own reader handles a rank-0 dataset - so the test asserts the stored shape instead. Reverted against the fix it fails with rank 0, dim 0 against the expected 5. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y5XisyYxmF8mUEQjzpMRe2 |
||
|
|
c1ebc2883a |
rugnux docs: give -N the rule the program actually follows
Three stages take fewer workers than -N asks for, and only one of them was described. The per-image cap is lifted by a positive -N; the first-pass spot finding and beam-stop pre-scan ceilings are not, so -N 32 does not give 32 workers there. A value <= 0 counts as automatic and is capped like the default, which the previous "an explicit value is used as given" contradicted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y5XisyYxmF8mUEQjzpMRe2 |
||
|
|
c570b9f3a8 |
rugnux docs: say which pass-1 files stopped being written, and which did not
"Only the refined pass is written" is true of the merged files and false of the process file: with --no-merge or --write-process-h5 each pass writes its own, so <prefix>_01_process.h5 is still there beside <prefix>_process.h5. Name the four files that did go, and the one that did not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y5XisyYxmF8mUEQjzpMRe2 |
||
|
|
3d30c1df95 |
HDF5 docs: describe the transformation chains this release actually writes
The detector depends_on table was the pre-rc.162 chain and was wrong in every row: the file now has rot3 at the root and translation at the leaf, with the detector depending on translation, which is what makes a tilt pivot about the sample. Give the vector values too, and say they are McStas rather than the internal frame - a reader cannot use the chain without that. Three prose paragraphs had been inserted into the middle of the non-standard- fields table, orphaning its last five rows into a second table headed bit_depth_image. Rows first, then the prose. The sample section still described a goniometer and a grid scan as alternatives and never mentioned the Smargon tagging, which is now load-bearing - a spindle can itself be called phi, and only equipment_component separates them. Also corrected: NXmxVDS is the default, not NXmxLegacy (FileWriterSettings.h, and the new SOFTWARE_INTEGRATION page already said so); the unsigned data-file fill is HDF5's own 0, not UINTx_MAX, since every unsigned code is a real count; underload_value is INTx_MIN+1 only for signed; and bit_depth_image equals bit_depth_readout only where the latter is written. The changelog claim about a grid scan reading back as one image is narrowed to the still-at-a-head-position case, which is the one the writer actually fixed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y5XisyYxmF8mUEQjzpMRe2 |
||
|
|
9b2b79aa73 |
rugnux docs: add the four options the table never listed
--overlap, --overlap-minpk, --prediction-mosaicity and --rotation-scale are all accepted by the program and two of them change what is integrated by default, but none appeared in the options table - so the only way to find them was to run the program with no arguments. Descriptions cut down from the usage text. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y5XisyYxmF8mUEQjzpMRe2 |
||
|
|
a5c58c3142 |
rugnux docs: cut the XDS-convention section to the size of what it describes
It is one log line and two gotchas, and it had grown a bulleted list and a closing paragraph on a par with the sections describing whole modes of the program. Same facts, one paragraph. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y5XisyYxmF8mUEQjzpMRe2 |
||
|
|
5aaff2adaf |
Changelog: collapse the rc.162 entries onto what a user does with them
The section had grown to 45 bullets, several of them the same functional change reported once per commit - four separate "faster" lines for one rotation run, the -S lattice fix split across three, the NXmx offset attributes split from the transformation chain they are attributes of. Group them by what changed for a user rather than by what was committed, and drop the incidental notes that belong in a commit message. 25 entries, each one line, in the order the release paragraph introduces them: the files other programs read, then rugnux, then speed, then the build. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y5XisyYxmF8mUEQjzpMRe2 |
||
|
|
ebf2064c26 |
Document the beam stop, the resolution estimate and where the ice score comes from
Three things the CPU analysis page either did not describe or described in the wrong place. The beam-stop finder had no entry at all, though it runs by default and puts pixels into the mask. It gets a short 1.5, next to the other measurement made from raw frames before anything is indexed: what it masks and why, the per-ring comparison the detection rests on, and the shaping that follows. The resolution estimate was two paragraphs on the end of "Resolution and ice-ring handling", which is neither where a reader looks for it nor what that section is about. It moves to its own 3.6 at the end of spot finding, where it belongs - it is read off the finished spot list - and loses the recapitulation of what the old estimator did wrong, which is commit-message material. The ice-ring positions are measured values taken from a paper, so they owe an acknowledgement, and had only an in-source comment. DOI verified against Crossref; the title is not the one the ring positions are usually attributed to. Also: the per-image loop's default worker cap is 16 per GPU, not the 8 the options table still gave, and it applies to --mode mx only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y5XisyYxmF8mUEQjzpMRe2 |
||
|
|
c470bed93a |
Allocate off the driver lock, and give the loop the sixteen workers a card wants
Build Packages / build:windows:nocuda (push) Successful in 16m31s
Build Packages / build:windows:cuda (push) Successful in 19m15s
Build Packages / build:viewer-tgz:cpu (push) Successful in 19m58s
Build Packages / build:viewer-tgz:cuda (push) Successful in 22m51s
Build Packages / build:rpm (ubuntu2404_nocuda) (push) Successful in 24m5s
Build Packages / build:rpm (rocky9_nocuda) (push) Successful in 25m3s
Build Packages / build:rpm (ubuntu2204_nocuda) (push) Successful in 28m51s
Build Packages / build:rpm (rocky8_nocuda) (push) Successful in 28m55s
Build Packages / build:rpm (rocky8_sls9) (push) Successful in 28m35s
Build Packages / build:rpm (rocky9_sls9) (push) Successful in 21m19s
Build Packages / XDS test (durin plugin) (push) Successful in 11m17s
Build Packages / build:rpm (rocky9) (push) Successful in 23m30s
Build Packages / Generate python client (push) Successful in 33s
Build Packages / build:rpm (rocky8) (push) Successful in 25m35s
Build Packages / Create release (push) Skipped
Build Packages / Build documentation (push) Successful in 1m37s
Build Packages / build:rpm (ubuntu2404) (push) Successful in 21m40s
Build Packages / XDS test (neggia plugin) (push) Successful in 10m17s
Build Packages / build:rpm (ubuntu2204) (push) Successful in 26m44s
Build Packages / XDS test (JFJoch plugin) (push) Successful in 11m10s
Build Packages / DIALS test (push) Successful in 23m27s
Build Packages / Unit tests (push) Successful in 1h19m59s
Two changes to how the per-image loop is set up, neither of which touches what it computes. Device buffers were taken with cudaMalloc and returned with cudaFree, both of which are on CUDA's implicit-synchronisation list: each one synchronises the device across every stream. One analysis engine per worker, each making a few dozen of them, means the workers still constructing stall the workers already processing images, and the cost grows with the worker count. They are now stream-ordered allocations from the device's memory pool, with the synchronous pair kept as the fallback where no pool is available. Two deliberate limits on that. The pool's release threshold is one gibibyte rather than unbounded: holding the small per-worker buffers is the whole point, but the card also has to fit the merge afterwards, which asks for several gigabytes of its own. And the shared geometry tables keep the synchronous allocator, because their deleter runs on whichever thread drops the last reference, so an asynchronous free there would be ordered on a stream that says nothing about the engine streams whose kernels read the table; they are allocated once per card, so the pool bought them nothing. The loop's worker cap per card goes from eight to sixteen. The comment beside it already recorded where the measurement put the minimum - the loop's time falls to sixteen workers and then rises - and a later measurement on a single card agrees: at eight the loop waits on the queue rather than on the card. An explicit -N is still obeyed as given. Reflection files are byte-identical on four crystals with the worker count doubled, which is the property the frame-ordered mosaicity smoothing and the deterministic prediction order were built to give. Thirty consecutive runs of one crystal on the pooled allocator: no failure, every file identical to the first. Peak device memory over the whole rotation test set is 4.9 of 16 gibibytes. Four minutes seventeen to four minutes one over thirty-eight crystals, each binary repeating itself to within half a per cent; nineteen crystals faster, nineteen level, none slower, and every column of the comparison table identical. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EGpGdgmJ8MyY9pCGWjktyi |
||
|
|
1d16dcd2b9 |
Read a chunk without zeroing it first, and hold no frame the fused decoder never writes
Build Packages / build:windows:nocuda (push) Successful in 16m8s
Build Packages / build:windows:cuda (push) Successful in 18m58s
Build Packages / build:viewer-tgz:cpu (push) Successful in 20m35s
Build Packages / build:viewer-tgz:cuda (push) Successful in 22m31s
Build Packages / build:rpm (rocky9_nocuda) (push) Successful in 25m9s
Build Packages / build:rpm (ubuntu2404_nocuda) (push) Successful in 25m6s
Build Packages / build:rpm (rocky8_nocuda) (push) Successful in 28m57s
Build Packages / build:rpm (ubuntu2204_nocuda) (push) Successful in 28m58s
Build Packages / build:rpm (rocky8_sls9) (push) Successful in 28m58s
Build Packages / XDS test (durin plugin) (push) Successful in 12m3s
Build Packages / build:rpm (rocky9_sls9) (push) Successful in 22m24s
Build Packages / build:rpm (rocky9) (push) Successful in 21m45s
Build Packages / Generate python client (push) Successful in 53s
Build Packages / build:rpm (rocky8) (push) Successful in 26m9s
Build Packages / Create release (push) Skipped
Build Packages / Build documentation (push) Successful in 1m37s
Build Packages / build:rpm (ubuntu2204) (push) Successful in 25m34s
Build Packages / build:rpm (ubuntu2404) (push) Successful in 22m0s
Build Packages / XDS test (JFJoch plugin) (push) Successful in 10m53s
Build Packages / XDS test (neggia plugin) (push) Successful in 9m29s
Build Packages / DIALS test (push) Successful in 23m40s
Build Packages / Unit tests (push) Successful in 1h20m1s
Three costs before and around the image loop. Every image allocated a fresh buffer for its compressed chunk and resized it, which value-initialises, and the read then overwrote every byte. At a few megabytes a chunk the allocation is large enough to be mapped rather than reused, so the zeroing was page-fault bound and cost more than the read it preceded - twenty gigabytes of it over a long sweep. The buffer now uses an allocator that does not construct, and the two HDF5 read paths are templated on the allocator so every existing caller compiles unchanged. The rebind is deliberate: without it the vector base rebinds to the default allocator and the zeroing quietly returns. The bitshuffle decoder allocated a whole uncompressed frame in its constructor - seventy megabytes a worker, five hundred and fifty across the loop - for the route that decodes the shuffled image separately. That route is taken only when a bitshuffle block is too large for the fused kernel, which neither writer this pipeline reads produces, so on a real frame the buffer is allocated, never touched, and freed. It is now allocated where it is used. The comment two lines below already warned against sizing a buffer from the uncompressed size; the line above it had not been given the same treatment. The first call into cuFFT pays the library's one-time initialisation, and it landed in the middle of the first pass with nothing to overlap it. It is now forced on a background thread at startup, alongside the file open and the mapping build, in the manner the shadow finder already uses. Finally, the detector mask was copied into the start message whether or not a file would carry it, which a merging run does not. It is filled where a writer is constructed - both places one is constructed, the second being the fallback that writes a process file when nothing indexed. Faster on eleven of thirty-eight crystals and slower on none; the whole rotation test set falls from four minutes thirty to four minutes seventeen, with each binary repeating itself to within half a per cent. Space groups thirty-five of thirty-eight and no failures throughout, and every column of the comparison table is identical. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EGpGdgmJ8MyY9pCGWjktyi |
||
|
|
641f890a40 |
Build the frame's constants once, and page-lock what the integration engine copies
The per-image geometry refinement is the largest stage of the image loop, and a third of it was arithmetic on numbers that never change. The residual derives the detector angles' sines and cosines, the goniometer's back-rotation - a three-argument hypot, a sine, a cosine and a division - and the reciprocal basis of the cell on every evaluation. On the rotation path the detector angles and the axis are held fixed and stored as plain doubles, so all of it is constant, not merely constant per block: there is one frame per image and one cell. Three solves an image, fifty iterations a solve and a thousand spots make it tens of thousands of repetitions of the same result. The frame's constants are now built once and handed in. The body they feed is the same body, split out rather than copied, so no expression is reassociated - in particular the reciprocal vector is still formed as the basis times the inverse volume, with the volume not folded into the basis. The spot confidence weights depend only on each spot's resolution and intensity, which no solver touches, and were recomputed identically for each of the three passes. They are computed once. The sort behind them ordered indices through a projection that chased a random eighty-byte-strided element per comparison; it now sorts a packed resolution and index, which makes the same comparisons in the same sequence and therefore the same permutation. The spot list itself was copied per image through an initializer list whose elements are const; it is passed as a view. The integration engine was the last one in the loop copying through pageable host memory - three transfers in and eight out per image, twenty-six bytes a reflection, while every other engine already page-locks its staging. A driver copy from pageable memory stages through its own pinned buffer on the calling thread, which is why an asynchronous copy was averaging a hundred and thirteen microseconds. Page-locked, the same seventeen thousand calls cost four hundred and thirty-two milliseconds instead of one and a half seconds, and the wait moves to the synchronisation point where it belongs. Two smaller ones: the reflections were copied into the per-image message for a process file that a merging run does not write, so the copy is made where a writer exists; and the intensity statistics and the Wilson estimate walked the same eighty-byte array twice to read twelve bytes, which is now one pass with each accumulation in its own order. Every reflection file is byte-identical on four crystals; the process file's reflections match dataset for dataset, and its azimuthal arrays differ no more between this build and the last than the last differs from itself. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EGpGdgmJ8MyY9pCGWjktyi |
||
|
|
9a50ae5a0f |
Take the asymptote where it is read, and count a shell's reflections on every core
Five costs in the merge, none of them arithmetic. The strong-reflection asymptote was estimated twice for every merge and the first estimate was read by nothing: no statement between the two touches the value, so wherever the resolution cutoff refits the error model the earlier one was thrown away. It is now taken once, at the point the number is reported. Its per-group scatter array was also built fresh on every call - fifty megabytes value-initialised on one thread and immediately overwritten - and now lives with the object. The merge accumulator did the same thing on a larger scale: ten arrays and a struct of accumulators, a quarter of a gigabyte in all, zeroed on one thread before the device wrote every element of them. The kernel is a grid-stride loop over all groups and writes all ten outputs unconditionally, so nothing was reading a zero it had put there. The arrays are kept and resized, and the unpack that follows runs over the cores instead of one; its only reduction is an integer count, which does not care in what order it is summed. The host path still clears, because it accumulates in place. Dropping collapsed frame scales walked every full at eighty bytes a record to read two fields of four. Both were already downloaded, so it reads those instead: a tenth of the traffic for the same answer. The error model's chi-square median was computed on every fit and printed once. It now keeps what the last fit used and takes the median where the line is written. Counting the reflections a resolution shell could hold walked the whole reciprocal box on one thread and built a vector of the survivors first. The walk is now split over the outer index with a per-thread tally summed in thread order, and the vector is gone. On the cells in the rotation test set that is four to eleven milliseconds a call against three calls a crystal; on a two hundred Angstrom cell it is ninety-three milliseconds down to five. A comment claiming the point-group pass is serial to fill the operator cache is no longer true and is corrected; the cache is filled by a parallel pass before it. Every reflection file is byte-identical on four crystals, under a pinned resolution limit and under the automatic cutoff - the latter being the configuration that actually exercises the moved asymptote, since a pinned limit never made the second call at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EGpGdgmJ8MyY9pCGWjktyi |
||
|
|
0109676bdf |
Keep the constants out of the dual numbers, and clear only the boxes
Two costs in the per-image loop, each measured before it was touched. Geometry refinement is the largest item in that loop - about half to two thirds of its processor time on the datasets where the loop matters, and all of it on the host. It runs three solves per image, and each one spends four fifths of itself inside the solver at barely two iterations: the cost is not convergence, it is what every residual evaluation does. The residual carried the blocks it does not refine as dual numbers, so each evaluation recomputed the two detector rotations, the whole orthogonalisation matrix, three cross products and the cell volume - all of them constant for the image - through the derivative machinery, several million times per run. Split the observed and predicted sides so the un-refined blocks pass as plain doubles, evaluate the cell side once when the functor is built, and let the rotator take a point whose type differs from the angle's. A dual number times a double is a dual number times a dual number whose derivatives are zero, so the arithmetic is the same one with the zeros removed. Integration cleared the owner and mask images for the whole frame before every image. On a large detector that is more than three hundred megabytes of writes to reset pixels of which about one in twenty-five is ever marked, and it cost most of what the integration kernels themselves cost. The marking kernel gained an unmarking mode - one kernel, so the two cannot drift apart - and the engine clears whichever way is cheaper for the frame in front of it, with a flag to force the full clear the first time and after anything threw. The size test is not decoration: without it, clearing box by box is slower than the memset on a small detector with many predictions, which is what the measurement said before it was added. Faster on thirteen of thirteen matched pairs: refinement by a quarter to a third, whole-run wall by one to eight per cent depending on how much of the run is the loop. The two changes pay in opposite regimes - refinement where the loop is processor-bound, the clear where the detector is large enough for the card to be the constraint. Every reflection file over seven datasets is byte-identical, and the solver did not merely land in the same place: it took the same path, agreeing digit for digit on iteration, residual and Jacobian evaluation counts. A new test runs two mismatched frames through one engine and compares against a fresh one, which is what a mark left behind would break. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016NNnL26LAvruQ9eLUUWvrJ |
||
|
|
b074cca6ea |
Put the reflections in the fixed group's own setting, or say they cannot be
Fixing a space group told the merge which absences to apply but never told it which basis to apply them in. Where the indexed lattice was already conventional the group was simply stamped on it, so a primitive tetragonal cell asked to merge in a C-centred orthorhombic group had that centring rule evaluated in a frame the reflections were not in, and half of them were declared systematically absent. The reindex that would have fixed this existed but was reachable only from the triclinic arm. So ask the character table for the group's own class. The Bravais search grows an optional class filter - one continue that skips characters of the wrong class, one answer of "none fits" when the metric cannot carry it - and the reindexing that followed the triclinic arm is lifted out and offered to a fixed group whose centring is not the indexed lattice's, mapping a trigonal-P request onto the hexagonal-P setting it is described in. With no class asked for, both new statements are dead and the search is what it was. That splits the failing cases in two, and conflating them was what made this wrong in both directions. A lattice that HAS a setting carrying the group is reindexed into it: the tetragonal case above recovers every observation it had been discarding, and a centred monoclinic one that had been merging from a primitive cell without any reindex - which nothing had noticed - goes from an error model that could barely be fitted to a healthy one. A lattice that genuinely has no such setting - a triclinic metric several degrees from monoclinic-C, or an F-centred cubic one asked for hexagonal-P, whose hexagonal description is R-centred - has no basis to be put in, and every statistic computed from it is meaningless. Those now stop, naming the group, its centring and the cell that was actually indexed, and they stop only after the reindex has been tried, so a mistyped but reachable group is repaired rather than rejected. The second pass keeps its existing flag-and-decline instead. Separately, the geometry pre-pass predicted in the primitive lattice only when no group was fixed. With a centred group fixed it integrated half the events, moved the error model, and shifted the post-refined distance by more than a tenth of a millimetre - enough, in a loop this sensitive, to send the second pass down the other branch. It now predicts primitive there whatever the group, which is what it already did de novo and which its discarded intensities have no opinion about; the one dataset this cost its indexing rate recovers completely, and lands on the same answer it reaches with no group given. Thirty-one of thirty-eight pinned runs are bit-identical and none is worse. De novo nothing changes at all, by construction and on the whole rotation test set. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016NNnL26LAvruQ9eLUUWvrJ |
||
|
|
e5442e7a07 |
Read the cell surface's twenty bytes, and give each candidate group a thread
Build Packages / build:windows:nocuda (push) Successful in 17m17s
Build Packages / build:windows:cuda (push) Successful in 19m29s
Build Packages / build:viewer-tgz:cpu (push) Successful in 20m4s
Build Packages / build:viewer-tgz:cuda (push) Successful in 22m1s
Build Packages / build:rpm (ubuntu2404_nocuda) (push) Successful in 23m37s
Build Packages / build:rpm (rocky9_nocuda) (push) Successful in 24m55s
Build Packages / build:rpm (rocky8_nocuda) (push) Successful in 29m14s
Build Packages / build:rpm (ubuntu2204_nocuda) (push) Successful in 29m17s
Build Packages / build:rpm (rocky8_sls9) (push) Successful in 30m37s
Build Packages / build:rpm (rocky9_sls9) (push) Successful in 19m23s
Build Packages / XDS test (durin plugin) (push) Successful in 11m38s
Build Packages / build:rpm (rocky9) (push) Successful in 24m1s
Build Packages / build:rpm (rocky8) (push) Successful in 25m59s
Build Packages / Generate python client (push) Successful in 50s
Build Packages / Create release (push) Skipped
Build Packages / Build documentation (push) Successful in 1m22s
Build Packages / build:rpm (ubuntu2404) (push) Successful in 21m14s
Build Packages / XDS test (JFJoch plugin) (push) Successful in 11m49s
Build Packages / build:rpm (ubuntu2204) (push) Successful in 26m27s
Build Packages / XDS test (neggia plugin) (push) Successful in 9m53s
Build Packages / DIALS test (push) Successful in 23m28s
Build Packages / Unit tests (push) Successful in 1h18m34s
Three unrelated costs in the tail, each measured before and after. The cell surface fits and scores by reaching into the fulls for sixteen bytes of an eighty-byte record, twenty-two times over. That is four times the traffic of the data it uses, and it was the whole of the cost: the arithmetic never was. One fused compaction of the intensity, sigma, correction, cell term and group, built in the passes that were already being made, and every later pass walks the compact array instead. Fit accumulation falls to a third, scoring to a third. What is left is the term build and the scatter, not the fit. The estimator is untouched. The space-group search hashed a reflection key per observation per candidate. The orbit representative is now interned to a dense index when the orbits are built, so the two tests that follow index an array. Both candidate loops also run a thread per candidate - the point-group loop, and the space-group loop, which was the larger of the two by far: a full pass over the merge with three absence tests and a map insert per reflection, once for every candidate group. The operator cache is filled by the serial pass that precedes them, so each candidate still sees the same operators, does its own arithmetic unchanged, and appends in the same order. The search runs in a third of the time. Writing the reflections was seventeen stream insertions per row for a quarter of a million rows. The rows are formatted in parallel blocks and written in order, in the same widths and precisions as before: the mmCIF in a tenth of the time, the hkl in a quarter. Faster on twelve of twelve matched pairs, eight per cent on the sum of minima, and region timers account for the wall clock to within five per cent. Every reflection file over the whole rotation test set is byte-identical, in both passes and all three formats. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016NNnL26LAvruQ9eLUUWvrJ |
||
|
|
9f26c5e8b1 |
Keep the per-observation corrections on the card that computed them
Run downloaded the correction factor for every partial, scattered it into an eight-and-a-half-million element array of eighty-byte records, kept a copy of it, filtered it twice on the host, gathered it back and uploaded it again. Five passes over six hundred and eighty megabytes, on the path where the data was already resident on the card. A comment above it warned that three host readers needed the scattered copy, and an earlier attempt read that as a reason to leave the whole thing alone. Taken one at a time the readers fall: both pass filters test quantities the card already holds - zeta and the frame index - so they become kernels over the correction array in place; the saved copy is now the download itself, thirty-four megabytes rather than a gather of the whole record; and the combine, the only genuine host reader, gets its own scatter immediately before it, which matters solely when observations are dumped. Zeta is compared in double on the device so the promotion matches the host's comparison exactly, and the drop count is an atomic add. The merge's own sweeps had the same shape: they walked the eighty-byte record to reach twenty bytes of it. They now build those twenty bytes once per merge, on all threads, and stream them. The reject median's first walk over every full goes entirely - the counts it was accumulating are the ones the error-model pass has already produced. The group histogram is one flat uninitialised buffer whose rows are cleared by the threads that use them, in place of a vector of vectors cleared twice, and the ingest no longer zeroes four hundred and fifty megabytes of staging that the following line overwrites. Faster on seventeen of seventeen matched pairs across two alternating sessions; eight to ten per cent of whole-run wall clock on the datasets where the tail dominates. The reflection files are byte-identical, including with a frame correlation cut and with an observation dump, which are what exercise the two filters and the host combine. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016NNnL26LAvruQ9eLUUWvrJ |
||
|
|
d19409f4a1 |
Resolve Eigen once, before Ceres, and refuse a build that mixes two
Ceres asks for Eigen with a version range - find_package(Eigen3 3.3.4...5 NO_MODULE) - and Eigen's own Eigen3ConfigVersion.cmake rejects any range whose endpoints differ in major version. A 3.4 Eigen therefore declares itself INCOMPATIBLE with Ceres' query, the search falls through, and Ceres binds whatever older Eigen comes next on the prefix path. Where a distro eigen3-devel 3.3.4 is installed alongside a 3.4 one, Ceres created Eigen3::Eigen first, in its own directory scope, pointing at the older headers, and exported it publicly; the project's own find_package then ran afterwards and made a second target pointing at the newer ones. Targets linking both - the geometry refinement and the scale/merge libraries - took the older Eigen first. The result was a binary holding Eigen 3.3.4 and 3.4.90 template instantiations at once. Identically mangled, they are merged at link time with disagreeing evaluator layouts, so the program is undefined: at -O2 it segfaulted inside an Eigen product under the lattice reduction, nine runs out of nine, and at -O3 it happened not to, which is luck rather than correctness. Resolve Eigen before Ceres is added. The first find_package to run creates the imported target and later ones leave it alone, so Ceres inherits ours. Then assert it: if Ceres ever creates an Eigen3::Eigen of its own, the configure fails with an explanation rather than producing a binary that is quietly ill-formed. The guard fires only on that condition, not merely because two Eigens are installed. After the change no translation unit sees the older headers - 0 of 227 flags files, against 30 before - and Ceres reports the Eigen it actually compiled against. The same nine runs that all crashed now all complete. Release output is unaffected: eight datasets give byte-identical reflection files and identical merging statistics either way, so nothing previously measured is invalidated. Eigen and ZLIB stay external find_package dependencies on every platform, and OVERRIDE_FIND_PACKAGE is not reintroduced. Where only one Eigen is installed - the Windows and macOS case - Ceres' non-range fallback honours the same Eigen3_DIR and the change is a no-op. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016NNnL26LAvruQ9eLUUWvrJ |
||
|
|
2f54a1189d |
Take the error-model split and the ASU grouping off one thread
Three regions of the merge tail, measured with instrumented timers and confirmed against a cycle profile. On a tail-heavy dataset the scale and merge tail is 70% of the run's wall clock at six of thirty-two logical cores busy, with the GPU idle 88% of the time, so this is where the CPU headroom is. fit_error_model ran a serial four-level nth_element cascade over the whole sample pool, twelve times per dataset. The two halves either side of a partition are disjoint and their contents are already fixed by the parent's nth_element, so the recursion can descend both at once; it now does while a range is worth a thread. The bins are unchanged. ComputeAsuGroups sorted indices with an indirect comparator, taking a cache miss per comparison into an array far larger than the last-level cache. It now sorts packed key-and-run pairs. Tie order does not matter because the packed key encodes h, k, l and the hand exactly, so every run in a tie reduces to the same reflection. The per-thread histogram prefix walked thirty-two separate histograms column-wise on one thread. It becomes a parallel per-group total, one sequential scan over two flat arrays, and a parallel hand-out of the bases - the same sums in the same order. Faster on 21 of 23 matched pairs in an alternating A/B, and on 15 of 15 in the quieter of the two sessions: 0.6% to 2.3% of whole-run wall clock depending on the dataset, around 1.8% in aggregate, and 3 to 4% of the time spent outside the image loop. The reflection files are byte-identical on every dataset tested. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016NNnL26LAvruQ9eLUUWvrJ |
||
|
|
54adcaafcc |
Estimate the resolution the merged data will reach, not the furthest spot found
The per-image resolution estimate was the 5th percentile of the spot d spacings - an extreme order statistic, so it measured where detection stops rather than how well the crystal diffracts. A large cell puts more reflections past the same threshold and scored better than a small cell that diffracts further; intensity was not used at all, so a weak crystal padded with spurious high-resolution detections ran away; and nothing clamped the answer to what the detector can deliver. Against the resolution the merged data actually reach it was 42% out in log-RMS, with 1 of 38 rotation datasets inside 0.2 A. Take instead the 1/d^2 beyond which 30% of the sum of sqrt(I) over the image's non-ice spots lies, report 1/(2.25 sqrt of it), clamp at the detector corner, and take the median over images. A quantile from the middle of the distribution measures the shape of the falloff - the crystal's own exp(-B/2d^2) - where an extreme one measures the threshold. sqrt(I) is the Poisson significance of a summed photon count, so a marginal high-resolution detection cannot carry the answer and neither can a handful of very strong low-resolution reflections. The 2.25 is the multiplicity gain: merging keeps measuring intensities a fixed factor in 1/d past the point where a single frame detects them. Spearman 0.881 -> 0.954, log-RMS 42% -> 8.9%, median error 0.79 -> 0.07 A, and 32 of 38 within 0.2 A. Both constants sit on a broad plateau, the scale is stable across dataset halves and across resolution ranges, and no second predictor survives leave-one-out. The residual is around 9%, set by multiplicity, symmetry and radiation damage - none of which a spot list can see. The estimate feeds only reporting: the image stream, HDF5, the plots, the scan result and the preview ring. It sets no cutoff and no search limit, and the scaling and merging output is byte-identical. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016NNnL26LAvruQ9eLUUWvrJ |
||
|
|
baf302f813 |
Map an FFT candidate to its own Bravais lattice, not to the pinned group's
A user-fixed space group reached indexing in one place, and what it did there was relabel a cell rather than re-express it. build_sr took the conventional cell that LatticeSearch had reduced for whatever Bravais class the METRIC matched, then overwrote its system and centring with the pinned group's - without transforming the cell. The constrained refine then snapped that cell's real angles onto the pinned class's ideal ones. Measured: a C-centred orthorhombic cell relabelled primitive monoclinic indexed 1 of 60 validation frames, and an F-cubic one relabelled trigonal indexed 0 of 60, where the same frames index 36/60 and 51/60 with no group given. The pseudo-symmetry guard was withheld at the same time - has_tri required no group - so the unconstrained cell did not exist, which also disabled the false-promotion rescue in pick_best. The only remaining outcome for a bad constrained cell was the throw. That is what decided the two centred-monoclinic cases, where the relabelling is a no-op and the metric really is the pinned class: the constrained solve runs out to the length bound at a fraction of 0.005 while the unconstrained solve on the same candidate reaches 0.7. The group names the symmetry; it does not say which basis the candidate came back in. It is applied where it belongs, to the scaling and the merge. Pinning each rotation test dataset to its reference group: 5 hard failures of 38 become none, and no dataset is worse than before. The de-novo path is unchanged by construction - with no group the deleted branch never ran, and has_tri's condition reduces to its old form - and a ten-dataset de-novo control reproduces the baseline exactly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016NNnL26LAvruQ9eLUUWvrJ |
||
|
|
bd2179b08f |
Let the parallel pass signal completion under the lock its waiter tests
RunTasks keeps its RunState - counter, mutex, condition variable - on its own stack frame and hands a pointer to the pool. The last worker to finish decremented the counter OUTSIDE the mutex and only then took it to notify, while the waiter's predicate was the counter itself. So the waiter could see zero the instant the decrement landed, find its predicate already true, never block, and return from RunTasks - popping the frame. The worker then locked a mutex and signalled a condition variable that no longer existed, writing pthread state into a frame the submitting thread had already reused. pthread_mutex_unlock writes owner and nusers as eight contiguous zero bytes. Land those on a live pointer and the next read of a member at offset 8 faults: the observed crash was fmt's buffer<char>::append with this == nullptr, in the log call immediately after a parallel pass, which is why it always appeared after the resolution-filter line - that line was simply the next thing to use the frame. Give the waiter a flag set under the same lock as the notify. Completion cannot then be observed until the notifier has released the mutex, i.e. after its last touch of the state. The counter keeps its lock-free fast path and decides only who notifies, so there is still exactly one lock per pass. Found independently by two investigations: a widened-window reproducer (2 crashes in 38 unfixed, 0 in 60 fixed; glibc's own "__owner == 0" assertion caught in the pool worker) and an isolated one that clobbered a freshly filled stack frame 732 times in 60000 and never after the fix. Growing RunState by eight bytes, changing nothing else, took the rate from 0/90 to 3 hard failures in 30. The dataset that failed about one run in twelve: 0 of 40. Space group and merged output unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
cf4fbcf96a |
Reduce the anomalous split once per ASU group, not once per observation
ComputeAsuGroups states the rule for itself - "one ASU reduction per distinct raw hkl (not per observation)" - and the anomalous split then did a gemmi ASU reduction and an unordered_map lookup for every one of the millions of fulls. Both things it wants are properties of the observation's ASU GROUP rather than of the observation: group_h/k/l is the group's SIGNED representative, so the same reduction applied to it returns the Friedel-merged key and the hand together. Reduce once per group into a dense accumulator indexed from there. The hand only follows the group when the merge distinguishes the hands; a Friedel-merged run holds both in one group and still has to ask per observation. SigAno and the merged statistics are unchanged (2.96 over 53303 acentric pairs, merge table byte-identical). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
29eda90190 |
Report the time the whole run took, not the last pass of it
processing_time_s is set inside RunPipeline, so it measures one pass. A rotation two-pass run integrates everything twice and the beam-stop / beam-centre pre-scan happens before either pass, none of which the reported number saw: on an 18 Mpx dataset it printed 7.58 s for a run that took 23.46 s. Time the run in Run(), where every pass is inside, and report that. The last pass is still printed alongside it when there was more than one, because the gap between them is what the second pass costs. The frame rate and throughput stay per-pass: they say how fast rugnux moves through images, which running a second pass does not change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
65d0d6f238 |
Note the faster geometry refinement and merge in the changelog
Build Packages / build:viewer-tgz:cpu (push) Successful in 18m57s
Build Packages / build:viewer-tgz:cuda (push) Successful in 21m30s
Build Packages / build:rpm (ubuntu2404_nocuda) (push) Successful in 23m3s
Build Packages / build:rpm (rocky9_nocuda) (push) Successful in 24m32s
Build Packages / build:rpm (ubuntu2204_nocuda) (push) Successful in 28m44s
Build Packages / build:rpm (rocky8_nocuda) (push) Successful in 28m55s
Build Packages / build:rpm (rocky8_sls9) (push) Successful in 30m19s
Build Packages / build:rpm (rocky9_sls9) (push) Successful in 21m5s
Build Packages / XDS test (durin plugin) (push) Successful in 11m50s
Build Packages / build:rpm (rocky9) (push) Successful in 23m21s
Build Packages / Generate python client (push) Successful in 34s
Build Packages / Build documentation (push) Successful in 1m31s
Build Packages / Create release (push) Skipped
Build Packages / build:rpm (rocky8) (push) Successful in 27m15s
Build Packages / build:rpm (ubuntu2204) (push) Successful in 25m43s
Build Packages / build:rpm (ubuntu2404) (push) Successful in 22m10s
Build Packages / XDS test (JFJoch plugin) (push) Successful in 11m12s
Build Packages / XDS test (neggia plugin) (push) Successful in 9m39s
Build Packages / DIALS test (push) Successful in 23m39s
Build Packages / build:windows:nocuda (push) Successful in 13m15s
Build Packages / build:windows:cuda (push) Successful in 15m35s
Build Packages / Unit tests (push) Successful in 1h20m17s
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
be75001833 |
Build the merge's per-frame quantities once per frame, and count what a shell can hold only when it is reported
Three passes over the ingested observations were doing more than they needed. The smoothed-geometry pass rebuilt a CrystalLattice and its three reciprocal vectors for every observation, each one a cross product and a cell volume, for a value that depends only on which frame the observation came from. On a large sweep that is tens of millions of constructions against a couple of thousand distinct answers. The completeness column counts how many unique reflections a shell could hold. It is read off a merge that gets written out, never off the ones the space-group search runs on the way there - and those are the expensive ones to count, because the search merges in P1, where the list is the whole hemisphere rather than an asymmetric unit of it. The keep flags were written over the whole observation array as 1 and then immediately over it again as 0 whenever a resolution limit is set, which the default low-resolution limit always does. They are filled once now. The merged reflections also get their capacity up front rather than doubling their way to it several times per pass. Merged intensities are unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
0d07141b5c |
Ask the geometry refinement for the derivatives it uses, and solve the normal equations
Four changes to the same least-squares fit, which the rotation first pass runs on every candidate lattice and the per-image path runs on every frame. The linear solver was DENSE_QR on a problem that is very tall and thin - thousands of spots against at most seventeen parameters. That is the shape QR handles worst: it copies the Jacobian out of Ceres' row-major storage into a column-major buffer on every solve, and Eigen's blocked Householder then degenerates to the unblocked path because its block size is the column count. Accumulating J^T J reads the Jacobian once instead. Both solve the same damped system, so the step is the same to round-off. Ceres sizes its dual numbers from the declared parameter blocks, not from which of them the caller then holds constant. Nothing outside a test set refine_distance_mm - the positional residual leaves the distance degenerate with the cell scale, which is why the rotation post-refinement fits it in a step of its own with the cell held fixed - so the block was declared only to be frozen, and every residual differentiated seventeen parameters to use sixteen. It is gone, along with the test that exercised distance recovery; that test seeded the distance off truth, which the cell would now absorb, so its seed moves to the true value. The post-refinement's own detector step held five of its seven blocks constant and now bakes them into the residual, leaving beam and distance. The predicted reciprocal vector was built by rotating all three direct columns and then crossing them. A rotation commutes with the cross product and leaves the triple product alone, so the same vector comes out of crossing the unrotated columns and turning the result once - three rotations become one, for every crystal system. The documentation described the arrangement before all this, and had drifted in a second way: the first-pass rotation indexing has been refining the detector tilt and the rotation axis by default, which the text said were held fixed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
4b48d19064 |
Keep the threads the parallel passes run on, rather than making them per pass
ParallelChunks and ParallelFor started a thread per chunk with std::async and joined it again on every call. A thread costs tens of microseconds to create and join, and the analysis code repeats some of these passes thousands of times in a run - the merge alone has dozens of call sites, several of them inside iteration loops - so a short pass could spend more on its threads than on the work. Both now run on a pool made once and kept. The split is unchanged, so a pass whose per-element work is independent still gives the serial answer bit for bit. Two properties the futures gave for free had to be kept explicitly. A pass reached from inside a pool worker runs inline instead of queueing, since the workers are occupied by the outer pass and waiting for one of them could wait forever. And an exception from any task is held until every task has finished and then rethrown to the caller, so the others still run to completion. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
f6983486c3 |
Put the test's mask in through LoadUserMask, not through a const_cast
The fused GPU preprocessor uploads a byte-per-pixel form of the mask that PixelMask derives when the mask is loaded. Writing the bitfield behind its back left that form stale, so the device worked from a mask the CPU reference did not have. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
aceaf8b714 |
Say what the program now does, in the places that still described what it used to
Build Packages / build:windows:nocuda (push) Successful in 13m21s
Build Packages / build:viewer-tgz:cpu (push) Successful in 26m8s
Build Packages / build:windows:cuda (push) Successful in 14m49s
Build Packages / build:viewer-tgz:cuda (push) Successful in 28m25s
Build Packages / build:rpm (ubuntu2404_nocuda) (push) Successful in 31m34s
Build Packages / build:rpm (rocky9_nocuda) (push) Successful in 32m10s
Build Packages / build:rpm (rocky8_nocuda) (push) Successful in 37m12s
Build Packages / build:rpm (ubuntu2204_nocuda) (push) Successful in 37m8s
Build Packages / build:rpm (rocky8_sls9) (push) Successful in 38m43s
Build Packages / build:rpm (rocky9_sls9) (push) Successful in 21m7s
Build Packages / XDS test (durin plugin) (push) Successful in 11m21s
Build Packages / build:rpm (rocky9) (push) Successful in 22m50s
Build Packages / build:rpm (rocky8) (push) Successful in 26m32s
Build Packages / Generate python client (push) Successful in 54s
Build Packages / Create release (push) Skipped
Build Packages / Build documentation (push) Successful in 1m46s
Build Packages / build:rpm (ubuntu2404) (push) Successful in 21m29s
Build Packages / XDS test (JFJoch plugin) (push) Successful in 11m36s
Build Packages / build:rpm (ubuntu2204) (push) Successful in 26m54s
Build Packages / XDS test (neggia plugin) (push) Successful in 9m39s
Build Packages / DIALS test (push) Successful in 22m39s
Build Packages / Unit tests (push) Failing after 1h26m26s
A pass over the user-facing text against the code, after a day of changes. Almost all of it is deletion. The pre-pass files: RUGNUX.md promised them twice, CPU_DATA_ANALYSIS.md once, the usage line for --rotation-no-postrefine, the prose in the run report, a viewer tooltip and a comment in the anomalous script. They are not written any more, so the promises are gone and one is replaced by a sentence saying what the pre-pass is for and that it writes no merged files. CPU_DATA_ANALYSIS.md's account of decompression said two kernels do the work and, specifically, that the kernel stages nothing in shared memory and is therefore indifferent to the bitshuffle block size the file declares. Both halves stopped being true when the decode was fused into the preprocessing: it asks for the block as dynamic shared memory, and a block larger than 16 kB falls back to the old two-kernel route. That fallback is a behaviour a reader needs, so it is now described rather than contradicted. RUGNUX.md said spot finding, integration and scaling run on the CPU and scale with -N. Spot finding, preprocessing, azimuthal integration, prediction and Bragg integration are on the GPU, and rotation scaling and merging are GPU-resident too; that sentence now names what runs where. The -N row gains the per-GPU cap, and the usage line it is documenting gains it too - the usage message is the authority, so the two now agree. Deleted a promise of a per-iteration scaling file that nothing has written since the standalone scaling tool was removed. The changelog keeps four entries for today, cut back to what the house rules ask for: what changed, one line, no measurements or mechanism. The reproducibility entry now says MERGED intensities and says plainly that the azimuthal profile is not yet reproducible in the same way, which is what the code delivers - the corrected ring sums are still float atomics, and neither dropping the corrections (solid angle ramps 63% across the profile) nor widening them (the per-warp accumulators do not fit in shared memory at the bin counts in use) turned out to be a way to fix it. The review also turned up two defects in today's own code - the per-image cost line reported the thread count rather than the workers the loop actually ran, and the cap reached the azimuthal and calibration modes, whose worker preprocesses and integrates on the CPU and wants every thread it can have. Both are fixed in the preceding commit, which touches the same files. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011n8riB6X59oRjkrSHzNPAU |
||
|
|
b380da2a91 |
Judge the refined pass against the pre-pass on the same footing
The two-pass guard rolls back to the header geometry when the refined pass looks worse than the pre-pass, and one of its three tests is a drop of more than 0.05 in CC1/2. Since the pre-pass stopped fitting its correction surfaces - it exists to pick a space group and post-refine the geometry, and its intensities are discarded - the two sides of that test were no longer measuring the same thing: the pre-pass's CC1/2 came out uncorrected and the refined pass's corrected. On one crystal here that flattered the refined pass by 0.008, and it is the wrong direction to be careless in, because it makes the guard slower to fire on a pass that really is bad. So measure the refined pass's CC1/2 before its surfaces are applied as well, and compare that. It cannot be had from the half-set accumulate alone, which was the cheap thing to hope for: CC1/2 correlates half-set means built on the error model's sigmas over the reflections the automatic resolution cutoff kept, so an accumulate on its own is a different quantity - and one biased low, which would make the guard fire too eagerly. It takes the same merge the pre-pass now does, without the statistics tail, before the surfaces run. That is one extra merge on the one pass that has surfaces, so a caller asks for it explicitly rather than paying for it by default: the two-pass driver does, --mode scale does not, because there is no other pass to compare against. Where the caller wants it but nothing was corrected anyway, the merge that already ran IS the uncorrected one and is reported as such; where nobody asked, the field stays absent rather than being filled in from the statistic that reads almost the same and is not. Also make the final in-symmetry merge unconditional, with P1 when no group was determined. The comment there has always said P1 stands in that case and the condition did the opposite, which would have written a search merge - zeta-filtered, ice-excluded, uncorrected - as the result. It turns out to be unreachable: with any reflections at all the search returns a group, because no symmetry leaves the identity point group whose representative is P1, a group with no screws or centering leaves a symmorphic candidate that has no absences to contradict and so is always eligible, and an empty merge throws in both engines before the search sees it. The two lines keep that promise here instead of resting on eligibility gates in another file that a later change could tighten without noticing what leaned on them; the reasoning is written at the site. Battery unchanged on all 24 crystals - same space group, reflection count and R_meas as the run before it - and the merged output is byte-identical on three crystals spanning the regimes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011n8riB6X59oRjkrSHzNPAU |
||
|
|
c133fd89a8 |
Give the per-image loop as many workers as the cards can take, not as many as the machine has
-N defaults to every hardware thread and the per-image loop spawned one worker for each of them. Every worker submits its own kernels to a card, and a card runs out of room to accept them long before it runs out of work to do: measured on two GPUs, the loop's own time falls from 10.76 s at four workers to 9.69 s at sixteen and then climbs back to 10.49 s at forty-eight. Forty-eight workers is slower than eight. The same shape appears on a small detector, with the turn further out because a frame is a smaller piece of work. So cap the loop at eight workers per card when -N was left alone. Per card, because that is what the queue depth belongs to; eight, because that is where the curve turns on the hardware this was measured on. Everything outside the loop - the merge, the surfaces, post-refinement - still gets the whole machine, because none of it is waiting on a card. An explicit -N is obeyed exactly as given, and the cap says so in the log when it fires. A previous attempt at this overrode an explicit -N and applied to the azimuthal and calibration modes as well, which is why it was refused; this one is only about the default. It matters most where it cannot be measured here. A two-card production node with 192 threads runs ninety-six workers per card against a curve that turns at eight, while this box at -N 48 across four cards sits at twelve and looks fine. Even so, on four cards the battery goes 6m28s -> 6m10s, with every crystal's space group, reflection count and R_meas identical to the run before it - the cap changes no arithmetic at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011n8riB6X59oRjkrSHzNPAU |
||
|
|
7762d8bd30 |
Build an observation only if the resolution range keeps it, and let the pre-pass skip what it discards
Two changes to what the scaling stage does at all, rather than to how fast it does it. Ingest converted every integrated partial into the eighty-byte record the scaling works on, and then threw away whatever fell outside the requested resolution range. On a large cell that is sixty-three million records built and fifty-seven million discarded - five gigabytes written, most of it to be skipped by every consumer afterwards. It now emits a twenty-four byte key per observation, sorts and buckets those, decides from the runs which raw hkl the range keeps, and builds the full record only for the survivors. The flux meter still sums a frame's whole background in that frame's own order on one thread, because it is the number every merged intensity is divided by; the first usable d is still taken over the run rather than over the survivors; and the sort order was already total, so how the keys are filled cannot change it. The other is the pre-pass. It exists to choose a space group and post-refine the geometry, and its merged intensities are discarded - the second pass makes them again at the refined geometry. It was nonetheless fitting the decay, absorption and modulation surfaces, measuring radiation damage and sweep quality, assigning R-free flags, converting to amplitudes, walking the observations again for R_meas, splitting the anomalous pairs and analysing twinning, all for a result nobody reads. A flag threaded from the call site turns that off on the pre-pass, following the convention the anomalous split already used. What the pre-pass keeps is what is read later: the merge itself, the error model, the resolution cutoff, and the whole per-shell statistics block - because the second pass is judged against the first, and that guard needs the pre-pass's completeness and CC1/2. The flag that says the statistics exist is untouched and still set unconditionally; moving it is what disabled the guard entirely in an earlier attempt at this, and with it the completeness bound, the CC1/2 bound, the lattice-conflict test and the fall back to the header geometry. One consequence to be aware of: the pre-pass's CC1/2 is now measured without the correction surfaces while the second pass's is measured with them, so the two are no longer compared on quite the same footing. It moves the guard in the direction of firing less readily, never more, so it cannot roll back a good pass - but it is a small loss of sensitivity and the next commit removes it. Merged output byte-identical on a large-cell set, a high-multiplicity one and a small one; battery 6m28s against 7m55s, space group unchanged on all 24 crystals. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011n8riB6X59oRjkrSHzNPAU |
||
|
|
ef0be2e52a |
Bound the offline lattice refinement by iterations, and stop rotating the same axis three times
Two things in the indexing path, one of them a reproducibility hole. XtalOptimizerData bounds a solve by iterations when it is told to and by WALL-CLOCK SECONDS when it is not, and its own header says why that matters: the same image refines to a different answer on a busier machine. The per-image refinement sets the iteration bound for exactly that reason. The rotation indexer never did, so its candidate-cell refinement ran under a one-second wall clock - three stages a candidate, up to eight candidates a scheme, twice a run. A run that has just been made reproducible from its prediction order to its accumulators was still free to pick a different lattice because the machine was loaded. It now takes the iteration bound offline and keeps the wall-clock one for a live acquisition, whose budget is real, which is the same split the per-image path already makes. The residual itself rotated the same axis three times over. It applies one orientation to three reciprocal-lattice vectors, and ceres::AngleAxisRotatePoint recomputes the angle, its sine, its cosine and the normalised axis on each call - and it does not inline at this optimisation level, so the compiler cannot notice. On a seventeen-parameter Jet each of those is a full dual-number evaluation. Computing the rotation once and applying it three times removes two hypots, two sines, two cosines, two divisions and six multiplies from every evaluation, which is about half the libm calls in it; hoisting a constant member's sine and cosine out of the same function takes two more. It runs everywhere the residual does - the indexer, the per-image refinement and the geometry refiner. Also lifts five SetParameterBlockConstant calls out of a per-observation loop in the detector solve, where they were executing once per observation to say the same thing. The rotation hoist was checked against the function it replaces on 200000 random dual numbers, including the small-angle branch, comparing the value and all seventeen derivative lanes: no difference in any component. Merged output is byte-identical on a 16 Mpx set and an ordinary one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011n8riB6X59oRjkrSHzNPAU |
||
|
|
cf2336e523 |
Decode a compressed frame into shared memory and preprocess it there
The image loop on a 16 Mpx detector is two thirds of the run and both cards are busy for essentially all of it, so card time removed is wall time removed. Of the six milliseconds a frame costs, two and a half were spent decompressing it - and not because the card was short of bandwidth. The LZ4 pass moved 53 GB/s where the strong-pixel flagger, reading the same image and the same bin table, gets 276. It is latency, not bandwidth: the copy loop moves 32 bytes per warp iteration with a syncwarp after each one, and for a match copy the source and the destination both derive from the same pointer, so nothing pipelines. The warp spends its time waiting for global memory, one dependent round trip at a time. So decode where the waiting is cheap. One CUDA block now owns one bitshuffle block: its first warp decodes the payload into shared memory, and the whole block then un-transposes and preprocesses out of shared and writes finished pixels. A shared round trip is tens of cycles rather than hundreds, and the 72 MB shuffled intermediate never reaches DRAM at all - the pair of kernels moved about 238 MB a frame and the fused one moves 93. The parser is lifted into a device function that both kernels call over the same bytes, so the standalone path and the fused one cannot decode a chunk differently. The statistics reduction had to change with it: 48 bytes of static shared on top of a full bitshuffle block costs a whole resident block per multiprocessor, so the counts now reduce through a warp shuffle and one integer atomic per warp. Blocks larger than 16 kB keep the two-kernel path, and the beam stop's own decoder is untouched. What this costs is decoder parallelism: a block that holds 16 kB of shared is one of four resident per multiprocessor on this card, where the old kernel fitted thirty-two warps each decoding on its own. The trade is favourable here and should be better on the production cards, which have half again as much shared memory per multiprocessor. Measured on a 16 Mpx rotation set at the production GPU count, with the indexing work of the next commit: 37.2 s -> 32.9 s, and the merged output is byte-identical. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011n8riB6X59oRjkrSHzNPAU |
||
|
|
f09fe4e2c5 |
Report the per-image cost as something that can be attributed
Each stage timer is wall time inside one worker, so it counts whatever that worker spent blocked - on the GPU, above all - as well as its own work. Dividing the mean by the worker count, as this did, assumes every worker was busy for the whole loop. Measured occupancy is a third of the workers asked for on a large detector and less on a small one, so the number people tune against came out low by that factor, and it moved with the contention rather than with the work. Report the share instead. A stage's fraction of a worker's own per-image time is what that stage is responsible for whatever the contention was, and spending that fraction against the loop's wall time per image gives a figure that is attributable and that sums to the loop. The worker mean is printed at the end rather than divided away, because the gap between it and the wall is the waiting, and the size of that gap is worth seeing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011n8riB6X59oRjkrSHzNPAU |
||
|
|
5ee0f22a61 |
Build the detector's lookup tables once, not once per worker
The image loop gives every worker its own analysis engine, so a run builds ninety-six of them. Each one derived, from scratch, tables that are the same in all of them: the byte-per-pixel mask, the resolution mask, the radial kernel, and the checksum that names the shared device tables. The checksum was the worst of it, because it is part of the cache KEY and so is computed before the lookup - a hit still hashed the whole table. On a 16 Mpx detector that is the bin table, the corrections and the mask, 126 MB an engine, about twelve gigabytes over a run, to answer a question whose answer had not changed. The header said it cost nothing measurable; a profile says otherwise, and says it is worst exactly during the ramp when the machine has nothing else to do. It cannot simply be remembered against the address, which is what it exists to catch: a buffer can be freed and another allocated where it was, and the cache would then hand back a device copy of something else. So the owner of the bytes computes it instead. The azimuthal mapping writes its two tables in its constructor and never again. The pixel mask re-derives its binary form and its checksum on every path that changes the mask, and all of those paths are now private to the class. The key therefore still describes the bytes as they are at the moment of the lookup. The resolution mask was two passes over every pixel - a float comparison into a vector<bool>, then a bit-by-bit repack - in each of the ninety-six. It is one pass now, writing the packed form directly, built once for the limits asked for and handed out as a shared pointer so a worker keeps the mask it was given. The radial kernel is cached on the six numbers it is derived from. Nothing computes a different value; only who computes it changes. Byte-identical merged output on a 16 Mpx set and on a small one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011n8riB6X59oRjkrSHzNPAU |
||
|
|
27020d27e9 |
Take the merge's per-observation sweeps off one thread
Of the seventeen seconds a high-multiplicity crystal spends in scaling and merging, only three are
GPU work. The rest is the host, and most of it was running on one or two cores of forty-eight.
Eight of those passes are elementwise maps over the observation array - restoring the scaling
correction at the start of a pass, saving it before the pass filters, scattering it back from the
device, the zeta filter, the frame rejection, the two gathers that hand it to the device again, and
the collapsed-scale ratio. Each reads and writes an eighty-byte record per observation, each ran
serially, and each runs once per cycle with five cycles in a run. They are independent per element,
so chunking them changes nothing but the wall clock. The zeta filter's drop count is now one atomic
add per chunk rather than per observation, and it is an integer, so no arrival order can move it.
The download of the combined fulls did the same work twice over: `assign(nf, Obs{})` zeroed a
quarter of a gigabyte that the next loop overwrote completely, fifteen scratch vectors were
allocated and zeroed afresh every cycle, and the gather from them was a three-million-iteration
serial loop. The scratch is now kept between cycles and the gather is chunked.
The correction surfaces were the last of it. Their inner pass sums the reference intensity of every
usable full, thirty-nine times a run, and a comment asked for per-worker accumulators if it ever
mattered. It does now, but per-worker accumulators would re-associate the double sums. The fulls are
already grouped by a stable counting sort, so walking that grouping visits each group's members in
increasing index - the order the serial loop added them in - and the sums keep their exact sequence.
Copying the four fields the pass actually reads into a packed record first is what makes it pay:
what kept this serial was not the addition but the random read across 265 MB of fat structs, and 53
MB read in order is a different thing.
Ingest is parallel over frames now, which is safe because a frame's mean background is still summed
in that frame's own order by one thread - it is the incident-flux meter and it has to be exact. The
larger rewrite it deserves, sorting a narrow key first and building the fat record only for the ten
per cent that survive the resolution cut, is left alone.
Measured with the surrounding commits: a high-multiplicity set 35.6 s -> 32.4 s, a large-cell one
52.6 s -> 46.1 s, byte-identical merged output on both.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011n8riB6X59oRjkrSHzNPAU
|
||
|
|
0fed94d75b |
Gather post-refinement's partials without touching four gigabytes twice
Post-refinement fits eight numbers, and it selects the twenty thousand best-recorded events to fit them from. Before it can select, it copies every integrated partial into an array of its own and sorts it. On a large cell that is 63 million of them, and the phase took 8.5 s of a 56 s run. Almost none of that was the sort. `std::vector<Partial> pts(n)` value-initialises: one thread writes 3.5 GB of zeroes, page by page, before the parallel fill overwrites every byte of it - and being the first touch, it also decides where the pages live, so the whole array lands on one NUMA node and every later pass over it runs at one node's bandwidth. The same again for the sorted copy. Allocate the storage without initialising it and let the parallel fill be the first touch. The record itself carried more than the sort reads. `angle_rad` is a function of the image number that the goniometer can give back on demand, and the two observed positions are wanted only by the distance step, and only for the twenty thousand it keeps. Storing what is read - and as the floats the fields already were, since widening a float to a double is exact - takes the record from 56 bytes to 32, which is a third off the fill and half off the sort's element moves. Then three passes that walked the whole array to no purpose. The h range is now taken in the count pass, which reads the same reflections anyway; the bucket histogram in the fill pass, which already has h in hand. The event split walked serially and grew its output by doubling - about a gigabyte of pure copying - although h is the leading sort key, so a rocking event never crosses an h bucket: count per bucket, prefix, fill in parallel, and the events come out in the order the serial walk produced them. And the copy of the whole event list, made only so that nth_element could destroy the original, is now an index array. Every one of these is the same arithmetic in the same order. Measured on a large-cell rotation set, with the two commits that follow: 52.6 s -> 46.1 s, and the merged .hkl, .mtz and .cif are byte-identical. `part_less` is deliberately left as it was, not a total order: what makes it reproducible is that each bucket reaches the sort in gather order, and the new chunking is still a contiguous span of that order. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011n8riB6X59oRjkrSHzNPAU |
||
|
|
4a537dbfc2 |
Bin the error model's samples without sorting them
The (a, b) fit wants sixteen equal-count bins in I^2 and takes three medians out of each. It was getting them by sorting the whole pool - millions of 32-byte samples - and it did that fourteen times a run: the fit runs once per merge and twice where the resolution cutoff refits, the outlier refit doubles it again, and there are five merges. Each call also took its pool BY VALUE, so every one of those began by copying tens of megabytes, and each bin then built three more vectors by push_back to hand to a median. A bin only has to be the right SET. Put each boundary in place with nth_element instead, splitting the boundaries down the middle so every level halves the range it works on - four levels of linear work against n log n - and take the three medians straight off the bin's own span with the field wanted, which is what median_of was doing anyway: it returns the lower median, exactly the element nth_element leaves at that index. No copy is made at all, and the sixteen bins are disjoint so they divide over the cores. The comparator is now total. The sort it replaces was not stable, so which of two samples of equal I^2 landed in which bin was decided by the order the pool happened to arrive in - and the refit is handed a different order from the first fit. Ordering on the remaining fields, which are in the same cache line, makes the bin a property of the samples instead. This is why the merged intensities are not byte-identical to the previous release on about half a percent of reflections, at a median difference of zero and a worst case of 1.2e-2: those are the ties, whose old resolution was arbitrary. Every fitted (a, b, ISa, chi2) in the run agrees to four significant figures. The per-group outlier median goes the same way. It was building a vector per ASU group to hold a handful of floats - over a million allocations, their growth and their frees, five times a run - where the counts were already to hand from the pass above. One flat array with a per-group span gives the identical median, since a median does not care how the multiset was laid out. Measured together with the previous commit on a high-multiplicity rotation set: 38.3 s -> 35.1 s. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011n8riB6X59oRjkrSHzNPAU |
||
|
|
c9fc46e6e2 |
Stop making the first pass write the files the second pass replaces
Pass 1 exists to choose the space group and post-refine the geometry. Its merged intensities are discarded - pass 2 remakes them seconds later at the refined geometry, and that is the answer anyone reads. It was nonetheless writing the full set of merged files at the end of every pass 1: a mmCIF of every unique reflection (22 MB on an ordinary crystal, 48 MB on a crowded one), an .hkl, an .mtz and the per-image scaling table, all through one thread. Measured on an ordinary rotation set: 0.60 s of a 15 s run, and pass 2's identical block right after it takes another 0.585 s to write the files that are kept. The pass-2 quality guard is untouched, which is what disqualified an earlier attempt at this: has_merge_statistics is set at the merge, well above the write, so pass 1 still reports the completeness and CC1/2 the guard compares against. Nothing numeric moves - the same run measures 38.5 s before and 36.6 s after with a byte-identical .hkl. Also hoist the pixel-mask accessor out of the preprocessor's per-pixel loop. It called .at() on every pixel of the detector - 18 million bounds checks per engine, and an engine is built per worker per pass - for a bound the loop already respects, which stopped it vectorising. The <prefix>_01.mtz/.cif/.hkl are documented output, so this is a deliberate behaviour change: the pre-pass result is no longer written. If it is wanted for comparison it should come back behind a flag rather than by default. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011n8riB6X59oRjkrSHzNPAU |
||
|
|
16639e9de2 |
Add up the profile accumulators in an order the schedule cannot change
Every accumulator in this file that could be an integer already is one, and the spot finder's reduce_rings_shared says why: a preprocessed pixel is an exact int32, integer addition is associative, and a threshold that moves in its last bits between runs flips every pixel sitting on it. Four accumulators here were still floats, and they reach the intensity rather than a diagnostic. * The radial background curve. s_radv and rad_sum sum int32 pixel values, so int64 is not an approximation of the old sum, it IS the old sum - and the curve is subtracted from every reflection's background. * The learned profile grid and its second moments, which are sums of (px - bkg) / I over every strong reflection of the frame. There is no exact integer form, so these are fixed point at 2^20: a quantum of 1e-6 of one I-normalised pixel, far below the Poisson noise of the pixel it came from, and some five orders of headroom inside a signed 64-bit accumulator. * The normalisation total in build_profiles, which divides every cell of the profile - 128 lanes on one address, in arrival order. Now summed as integers, exactly, from the grid it normalises. * The fit's own reductions, s_num and s_den among them, which ARE the fitted intensity. These stay float, so fixed point would be a real precision trade over an unbounded range; instead each warp leaves its total in a slot of its own and every thread adds the slots up by warp index. WARP_ATOMIC_ADD is order-independent for the integer accumulators it was written for and not for these, which is what block_sum is for. With the prediction ordering of the previous commit, a run is now reproducible: the same command on the same images writes byte-identical .hkl and .mtz, at -N 1 and at -N 48, on a 16 Mpx rotation set and on a large-cell one. Before, all four differed. The battery is unchanged where it was ever stable: space group identical on all 24 crystals, reflection count on 20, R_meas on 22. The two that move are the two the battery has always seen move between runs of an unchanged binary - which is the point, since they stop moving now. Total 8m02s against 7m55s, inside the noise of per-crystal times quantised to a second. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011n8riB6X59oRjkrSHzNPAU |
||
|
|
484a0e162a |
Give the predicted reflections an order of their own
The GPU predictors claim their output slot with atomicAdd(counter, 1), so a reflection's position in the array is whatever order the blocks happened to finish in. That position is not private to the predictor. BraggOwnerKey packs it into the owner map as the tie-break between two centres equidistant from a shared pixel - the map's atomicMin is order-independent, but the number it compares is not - and the ingest and post-refine bucket sorts, whose comparators are deliberately not total, resolve their ties by the order they are handed. So two runs of the same binary on the same images integrated a different set of reflections. Measured on a large-cell rotation dataset: 63301112 observations against 63301139, and 89% of the merged intensities differing by more than 1% of themselves, median 1.8%. Single-threaded as well as at -N 48, which is what ruled out thread ordering and pointed here. Order the downloaded list by (h, k, l, delta_phi) before TruncateToOutput, whose own pick is then reproducible as well. hkl is a property of the reflection rather than of the schedule, and delta_phi separates the two rocking solutions one hkl can have. The CPU predictors already emit in hkl order, so the two paths now agree on it. Sorting a 20-byte key and gathering once, rather than sorting the 88-byte reflections in place, keeps this off the clock: on a crystal predicting some 35000 reflections a frame the run measures 52.2 s against 52.3 s before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011n8riB6X59oRjkrSHzNPAU |
||
|
|
c3236eed44 |
Publish the underload, not the error marker, on the finalized-file socket
The finalized-file notification has carried j["underload"] = error_value since the two meant the same thing. They stopped meaning the same thing when error_value became the marker the pixels actually store: UINTx_MAX for an unsigned image, where it used to be GetUnderflow()'s -1, a value no unsigned pixel can hold and which therefore excluded nothing. So for an unsigned 16-bit run the key went from -1 to 65535. A facility that forwards it into an XDS UNDERLOAD or a DIALS trusted range - which is what a key called "underload" is for - would reject every pixel below 65535, i.e. all of them. Nothing in HDF5 is affected and none of the writer tests look at this socket, so it fails silently and outside the file. The start message already carries underload_value, the lowest valid value: 0 for an unsigned image and INTx_MIN+1 for a signed one. Send that. The signed case moves too, from the marker itself to one above it, which is what the key has always claimed to be. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011n8riB6X59oRjkrSHzNPAU |
||
|
|
0cca6945e2 |
Find the first pass's spots on every worker
The first pass built one analysis engine and walked its candidate frames through it serially, while the main image loop had been giving an engine to each of its workers all along. On a 16 Mpx dataset that phase was 21% of the run on one thread and one card. A frame's spots are a pure function of that frame and the settings - the engine carries nothing from one image to the next, which is exactly why the main loop can hand one to every worker - so the search parallelises without changing anything it finds. Each worker keeps its engine for the whole pass and takes a card by index, so an engine always meets the card it was built on, and the engines are released before the main loop builds its own; the peak is no higher than the main loop already reaches. Results land in a slot indexed by position and are inserted into the cache afterwards by the owning thread, and the feed loop still walks ordinals in order and still stops on the same condition. So neither a frame's spots nor the set of frames the indexer sees depends on how the workers interleaved: the lattice picked is the same one, on the same frames. Ported from 2608-performance with its two unrelated riders - the flag_strong quad read and the beam-stop shard allocation - split into commits of their own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011n8riB6X59oRjkrSHzNPAU |
||
|
|
9b1ffbaa71 |
Allocate a beam-stop shard's accumulators when it is first used
SetShardCount allocated and zeroed three per-pixel accumulators for every shard up front. With a GPU present none of them is ever written - the frames are decoded and folded on the device - and on a 16 Mpx detector eight shards are 2.9 GB to allocate and clear, measured at 0.8 s of the pre-scan spent on memory nothing reads. A shard now allocates on the first frame that reaches it, and the fold skips shards that never got one. Two things that go with it, not in the version on 2608-performance. Reduce's single-shard fast path returns that shard directly, which is now an EMPTY projection if nothing was ever added to it, where before it was a zeroed full-size one - and both callers index it by pixel. The fast path therefore requires the shard to hold at least one frame; otherwise the general path builds the zeroed projection as before. Also drops a duplicate include of ParallelFor.h. Split out of "Find the first pass's spots on every worker", which carried it as an unrelated rider. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011n8riB6X59oRjkrSHzNPAU |
||
|
|
0047d1065e |
Read four pixels at a time when flagging strong pixels
The ring reduction already reads its pixels four at a time; the pass that flags the strong ones still read them one at a time, over the same image. Give it the same quad read. The flag is a per-pixel comparison against a threshold the reduction has already fixed, so nothing is summed here and the result is unchanged pixel for pixel, including the tail the quad read does not cover and the masked pixels it skips. Split out of "Find the first pass's spots on every worker" on 2608-performance, which carried it as an unrelated rider. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011n8riB6X59oRjkrSHzNPAU |
||
|
|
74e8b77a0d |
Stop scaling and merging what the resolution range excludes
A crystal integrated to the detector corner but merged well short of it carries observations through the whole merge that the merge then discards. On the heaviest dataset in the rotation test set that is 63.3 M partials of which 6.4 M are ever used: the other nine tenths are sorted, uploaded, scaled, combined and error-modelled before anything looks at their resolution. Ingest copied every one of them unconditionally, and the d_min limit was first applied far downstream, in the ASU grouping. They are now dropped at ingest, immediately after the one big sort: - WHOLE raw-hkl runs are dropped, on the same rawrun_d the ASU grouping already tests. A per-observation test is not equivalent - a run is in or out today by one member's d - and using a different rule here would put the two out of step. - The drop happens AFTER the flux meter, which takes each frame's mean background over every reflection on it, and after the sort, so neither changes. - The compaction runs in index order, so a frame's observations stay contiguous and keep their order, and every per-frame sum keeps its sequence of roundings. The incident-flux divide goes with it: it was reading one int and dividing one float across 5 GB in a pass of its own. The per-frame mean it needs is now accumulated by the ingest fill loop - one frame, one thread, same order, so bit-exact - and the divide rides on the finiteness pass that already touches that field. Ported from 2608-performance with two changes. The ingest fill loop there had been parallelised by an earlier commit that is not being taken, so the mean background is accumulated in the serial loop this branch still has; it is the same sum in the same order either way. And the post-refinement sampling that commit also introduced - thinning the fit to 8 M partials by a hash of the raw hkl - is NOT included. Every consumer of the dropped observations is gated on the ASU group, so dropping them is a no-op for the science; thinning post-refinement is not, its own measurement puts the cell scale breaking at 4 M against a pool of 8 to 16 M, and it makes the fit depend on how far integration ran. That belongs to its own decision. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011n8riB6X59oRjkrSHzNPAU |
||
|
|
be73288748 |
Fit the modulation surface on a grid that spans the detector
The detector-frame modulation correction takes its 16x16 grid extent from a pass over every full, but the surface is fitted only on the fulls that belong to an ASU group. Those are two different populations, and the gap between them is whatever was integrated past the resolution the merge uses. That made the correction's fate depend on how far integration reached. Cut it back and the grid contracts onto the merged disc while the cell count stays the same, so each cell holds too few reflections, the surface over-fits, and cross-validation throws it away - correctly, on a surface that should never have been fitted at that scale. Varying only the integration limit on one rotation dataset, merged R_meas came out 28.4 / 33.1 / 29.0 / 32.8 / 31.7 %, and the four-point spread is entirely the correction switching on and off: every low value is a run where it was applied, every high value one where it was refused, with no exceptions. Nothing else moved. The grid now spans the detector. Cells with no observations in them keep a factor of 1 and cost nothing, and with integration running to the detector corner - the default - the grid is the one it always was, so the common case is unchanged. On a crystal carrying no resolution limit at all it takes merged R_meas from 39.7 % to 35.5 %. This is a correctness fix in its own right. It also has to come first: without it, any change that narrows the integrated resolution range trips the same over-fit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
|
2eb9780fe2 |
Weight the corrected intensity, not its factors, in the shared reference
Folding the fit loop and the score loop into one reference() had to pick one of their two spellings, and it picked the fit loop's: w * I * corr * a, where the score loop had built Is = I * corr * a first and then summed w * Is. Those differ in the last place, and of the two callers it is the score that decides whether a surface is kept at all - so a gate sitting on the fence could go the other way for no reason but the order of three multiplications. Sum w * Is, which leaves the deciding path spelled as it was and matches how the rest of this file accumulates a weighted intensity. The fit's own reference moves by a last place instead; it is iterated to convergence and then scored, so that is the cheaper place to absorb it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011n8riB6X59oRjkrSHzNPAU |
||
|
|
ca3ca7170e |
Spread the scaling corrections and the space-group search over the cores
Two thirds of a rotation run is one thread. The image loop is not the problem - on the heaviest crystal of the battery it is 1.8 s of 40 - and neither GPU nor CPU is saturated, because while the corrections and the space-group search run there is one core working and 47 idle. Mean occupancy over the whole run: 3.9 of 48. In the correction surfaces (absorption in the goniometer frame, detector-plane modulation, absorption against time and detector position - all one function): the per-cell accumulation, the score reduction and the final apply are now chunked, as are the three loops that assign a full to its cell, one of which spends a sine and a cosine per full de-rotating it into the crystal frame. Two full sorts of four million floats went with them: only the nine bin edges are wanted, so they are selected instead, each selection starting where the last one left off. The per-group pass is deliberately left serial. The terms of one group are spread all over the list, so the only way to give a thread groups of its own is to walk in group order, and that trades a near-sequential read of the fulls for a random one over a few hundred megabytes - the trade that already lost once in the combine kernel. The space-group search scores each candidate rotation by correlating I(h) against I(Rh) over the whole merge. Every operator it can ask about comes from a fixed list and none of them depend on each other, so they are scored up front, in parallel, and the search reads the cache. The scratch that stops a pair being counted twice is now per worker rather than shared. Worker counts are gated on how much work there is, not on how many cores the machine has (ThreadsForWork). Both parallel helpers start a thread per chunk, so a small dataset on a large node would otherwise pay for 48 thread starts to sum a few thousand terms - and this runs on 8-core laptops as well as on this node. Measured on the heaviest crystal, idle machine, two runs each, summed over both passes: those phases go 7.88 s -> 5.19 s. Whole-run wall time is the wrong ruler for it - it moves +-4 s between identical runs. Battery 9m45s -> 9m23s, space group 21/24, no failures; 16 of 24 crystals bit-identical to the previous run and the rest inside the noise floor of running one binary twice. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
|
01b619cfd0 |
Take the double precision out of the box integrator's inner loops
boxsum summed its ring background in double and compared each ring pixel against a double threshold. The pixels are integers: a sum of at most a thousand int32 values is exact in a 64-bit integer AND exact in a double, so the two agree bit for bit, and comparing an integer against the floor of the threshold accepts exactly the same pixels as comparing it against the threshold itself. Both loops now do integer arithmetic. That was 39% of the card's double-precision pipe on the development machine and about three quarters of it on the production one, where the double rate is unchanged from Turing while the single rate has doubled - so this is worth more there than here. Alongside it, three things in the combine kernel. rr_nusable was computed by a whole extra walk over every observation and then never downloaded or read by anything. sum_wb and sum_cwb have no F in them, so they are the same in all three reweights and only the last round's values are ever used - two thirds of them were two divisions each, discarded. And CombineParams was the one parameter struct in the file without __restrict__, so the compiler could not assume the observation arrays and the freshly allocated fulls arrays were distinct. Measured on a crystal with 66 million partial observations: boxsum 12.2 s -> 8.3 s, the combine kernel 8.0 s -> 7.6 s, whole crystal 1m17s -> 1m12s. Battery 15m32s -> 9m59s. Same space group on all 24 crystals, none failed. Two things measured and NOT kept, recorded so they are not tried again: sorting the raw-hkl runs by length so a warp holds runs of similar length - it trades away the locality of neighbouring runs in the permutation and came out slower (7.6 s -> 8.8 s); and page-locking the integrator's host staging arrays individually - eleven separate registrations of small heap allocations overlap on shared pages and the driver refuses them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
a8cca3e5d4 |
Parallelise the incident-flux divide, drop a redundant sync
DivideOutIncidentFlux was still the last fully serial pass in Ingest: a sweep over every observation to take each frame's mean background, and another to divide every rlp by its frame's flux. Ten gigabytes of traffic on one thread. The per-frame means go a frame at a time rather than an observation at a time, so each frame's running sum stays in one thread and in the order it had - splitting by observation would cut a frame across two threads and the partial sums would have to be recombined, which is a different sequence of roundings. The divide is per-element and splits anywhere. The adaptive spot finder synchronised after flagging strong pixels. The extractor that reads those pixels runs on the same stream, so the ordering already guaranteed the flagging had finished; the wait only idled the host, once per image. Measured on a crystal with 66 million partial observations: Ingest 8.5 s and 7.7 s -> 7.1 s and 6.6 s, whole crystal 1m24s -> 1m17s. Merged statistics unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
8ae53b7fdf |
Say what actually makes the post-refine bucket sort reproducible
The bucketing commit justified itself with "the partials order became total in an earlier commit", which is true of the scale/merge ingest and not of this sort: part_less ends at the image number, so two partials of one reflection on one image tie, exactly as they did before. Nothing is wrong with the result. The counting-sort prefix lays each bucket out chunk by chunk, and a chunk is a contiguous span of the gathered order, so every bucket arrives at std::sort in global gather order no matter how many threads scattered it - the order is reproducible run to run and identical across -N. Giving Partial a rank field to make the comparator total would settle those ties by index instead, at eight more bytes on an array that reaches tens of millions of elements, and would change nothing anyone can observe. So state the invariant where the comparator is, rather than leaving the next reader to trust a claim that does not hold for this half. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011n8riB6X59oRjkrSHzNPAU |
||
|
|
01d16231b3 |
Sort the partials and the post-refine events in buckets, in parallel
Both were one std::sort on one thread over tens of millions of elements, and together they were a third of a crowded crystal's run. Bucketing by h first makes them parallel. h is the comparator's leading key, so the sorted array is exactly the buckets laid end to end, and each bucket sorts on its own thread. In Ingest the keys are built straight into their bucket slot, so this replaces the build pass rather than adding one and the packed-key array is never duplicated; the extra memory is a few hundred kilobytes of histograms. Buckets are taken largest first, because the tail of the phase is whichever bucket finishes last. The run split falls out of the same structure for free: a run of equal (h,k,l) never crosses an h boundary, so each bucket counts its own runs, a scan over the buckets gives the offsets, and the arrays are sized exactly - which also removes the repeated growth the push_backs were paying for. The h range comes from the finiteness pass, which already reads every observation. The partials order became total in an earlier commit, when the observation index was added as the last key. That is what makes this safe rather than merely fast: the permutation is uniquely determined, so a bucket sort produces the same one a single sort would. Measured on a crystal with 66 million partial observations: Ingest 15.2 s and 14.3 s -> 8.3 s and 7.4 s, the post-refine event sort out of the top ten gaps entirely, the whole crystal 2m22s -> 1m24s. Battery 15m32s -> 10m05s. Same space group on all 24 crystals, none failed, and no crystal's R_meas moved by more than 0.3 points. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
c1b85c7e88 |
Reduce within the warp before the Bragg integration atomics
fit and boxsum were 80% of GPU time on a crowded crystal - 78 s of it. Neither was bandwidth- or occupancy-bound: both sat at about an eighth of the issue rate the card can sustain, stalled. What stalls them is the block-wide accumulations. Every one has all 128 lanes of the block adding into one shared address, and a shared-memory atomicAdd on a float or a 64-bit integer has no instruction on either Turing or Ada - it compiles to a compare-and-swap retry loop. So those 128 lanes serialise into 128 retries, eighteen times per thread in fit. Summing across the warp first and letting one lane do the atomic leaves four per block instead of 128. That is the whole story: the arithmetic below was worth 2%, the atomics 5.6x. The arithmetic is still worth having, and is what was expected to matter: - compute_shell ran on all 128 threads of a block for a value that belongs to the reflection. It is two software double-precision divisions, on a card whose double throughput is a thirty-second (a sixty-fourth on the production one) of its single. One thread does it now. - The Kabsch inner loop divided by the same weight three times; the compiler emits the whole correctly-rounded sequence each time. One reciprocal now. Likewise the two Gaussian widths and the profile normalisation, which are constant over a reflection's cells and were divided per cell. - boxsum read the pixel before deciding whether it wanted it. The window is the bounding box of an ellipse, so nearly half of it is neither the signal disk nor the background ring, and those slots were fetching a cache line for nothing. Measured: fit 50.8 s -> 9.1 s, boxsum 27.5 s -> 12.1 s. A crowded crystal 2m22s -> 1m58s, a 16M-pixel one 39.5 s -> 37.2 s, the whole battery 12m30s -> 11m35s. Same space group on all 24 crystals, none failed. The integer sums are unchanged - addition is associative. The float ones move in their last bits and become more reproducible, since a fixed shuffle tree replaces whatever order the atomics arrived in. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
52756273e1 |
Make the partials order total, and hoist 1/sigma out of the IRLS loop
The sort that orders every observation by (h,k,l,image_number) was not a total order: two observations can genuinely share all four. The predictor emits BOTH intersections of a reflection's rotation circle with the Ewald sphere, and near the blind region - where zeta is smallest - the two are close enough in angle that both are accepted on the same frame. Which of them came first was then whatever the sort happened to produce. That was observable. The combine takes on_ice from the FIRST member of a rocking event, so the order decided whether a full was flagged as ice at all, and its per-event sums are floating point, so it moved intensities in their last bits. The observation's own index is now the final key, which orders them by arrival - and, more usefully, makes the order unique, so it no longer depends on which algorithm sorted it. sigma never changes once it is uploaded, so 1/sigma is the same in all thirty IRLS iterations of all three scaling iterations of all five scaling passes. It was being recomputed every time: a 64-bit reciprocal is a hardware estimate plus five refinement steps, and the profile put the three divisions in that loop at 21 of its 31 double-precision instructions. It is computed once now, in the pass that already streams every observation. The CPU has always hoisted it; this is the GPU catching up. Same expression on the same operand, so the value is what the loop used to compute, bit for bit. Also: PrepScaleObsKernel is not a grid-stride loop, but the scale-fulls path capped its grid at 65535 blocks like the grid-stride kernels around it. Above 16.8 million fulls that silently left the tail of sco_coeff/sco_ok stale. No dataset here reaches it; the cap is simply wrong for that kernel. And the AoS-to-SoA staging that feeds the GPU - the widest pass in Ingest, reading an 80-byte struct and writing fourteen arrays out of it - ran on one thread. Full 24-crystal battery: same space group on all 24, none failed, one crystal moved R_meas by 0.8 points with CC unchanged (it moves by that much between runs of an identical binary). 15m32s -> 13m35s. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
594100accc |
Make the rotation-scale fit reproducible, and stop refining past a float
Two follow-ups to the closed-form fit. The five per-fifth sums were reduced under a mutex, so the order in which the chunks were added depended on which worker reached the lock first and the fitted scale moved in its last bits between runs of the same binary. Each chunk now folds into its own slot and the slots are summed in chunk order, which is the reduction pattern the rest of the analysis code uses. The split ParallelChunks makes is fixed, so the sum is now the same sequence every time. The golden section bracketed to 1e-9. The fit is narrowed to a float before it is applied, and a float's epsilon is 6e-8, so the last ten or so iterations - each a full parallel pass over every event - refined digits that are discarded on the next line. Bracket to 1e-7. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011n8riB6X59oRjkrSHzNPAU |
||
|
|
c4c2d598d7 |
Fit the goniometer rotation scale in closed form
The fit has ONE parameter, and it was handed to Ceres as one residual block per rocking event - 8 million of them on a large crystal. Each block is a functor, an auto-diff cost function and a loss object on the heap, and the solver then factorises an 8-million-by-one Jacobian on every iteration. It cost 13.7 s. The residual is closed-form in k. A rotation preserves length, so |p_lab| is |e_mid| whatever k is and only the z component moves; Rodrigues gives it exactly: r(k) = C + A cos(a k) - B sin(a k) = C + R cos(a k + psi) C = lambda |e|^2 / 2 + u_z (u.e), A = e_z - u_z (u.e), B = (u x e)_z with a the event's angle from the sweep centre. That is the same function the functor computes - Ceres uses the exact Rodrigues form here, so there is no small-angle branch to disagree with - and it reduces the fit to minimising a smooth function of one variable over the interval the solver was bounded to. It is scanned on a grid and then closed in by golden section; the objective's curvature jumps wherever an event crosses the Huber knee, which is why this is not a Newton iteration. The coefficients are computed in double and stored narrowed. Their rounding moves the minimiser by ~1e-10, and k is carried downstream as a float, so the committed value is the same to far more digits than anything reads. One pass over the events yields the five per-fifth partial sums, so the all-data fit and the five leave-a-fifth-out folds share it. That matters because the jackknife only runs when the fit is big enough to act on, and on a crystal that trips it the old code paid for six full solves. The partials gather ahead of it counted first and then filled instead of growing one vector by push_back tens of millions of times, which copied the whole thing on every doubling. Measured: unchanged verdict and k to five decimals on the regression crystals. Full 24-crystal battery: same space group on all 24, none failed, 15m32s -> 13m35s together with the scale/merge changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
c3d3161af0 |
Fold the beam-stop batch before its frame size changes
ShadowAccumulatorGPU::Add sized the raw buffer before closing the pending batch, and EnsureRawCapacity assigned frame_bytes on entry. FoldPending strides `raw` by frame_bytes, so a frame of a different size arriving mid-batch made the already-decoded frames fold with the new stride: every pixel of the pending batch read from the wrong offset, silently, with no error. The depth-change branch that exists to handle exactly this ran one step too late to help. Fold first, then resize, then adopt the new stride. The batch also closes on a change of frame size, not only of pixel mode - a batch is one layout, and the mode alone does not fix the layout. The decoder was likewise built once from the first frame and never rebuilt, so it is now rebuilt when the frame size changes; without that the mixed-size path this commit repairs would still decode into a buffer of the wrong size. Also calls Gpu() once in ShadowFinder::AddImage instead of twice - it takes and releases a mutex each time, once per image. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011n8riB6X59oRjkrSHzNPAU |
||
|
|
ddf625d833 |
Decode and accumulate the beam-stop projection on the GPU
The pre-scan decompressed its frames on the host and folded them into a per-pixel projection there. On a 16M-pixel detector that is 60 frames of 72 MB to decompress and 20 bytes per pixel to read and write back per frame - about 40 GB of memory traffic - and it was the whole cost of the phase once the mask was no longer the bottleneck. Only the compressed chunk crosses PCIe now. BSLZ4DecoderGPU already exposes the raw decoded bytes (Decode(), the path its own tests use), which is what this needs: the projection is defined on the RAW STORED COUNTS with the pixel type's sentinel skipped, not on the preprocessed image, so nothing here goes through the preprocessor. Sums, maxima and counts are integers, so the device result is identical to the host's rather than merely close. Frames are folded in batches of four. The fold reads and writes the whole accumulator whatever the batch holds, so per frame it was spending most of the bandwidth on the accumulator rather than on the data; four is where that stops mattering, and every frame beyond it is another full frame of device memory, which costs more in cudaMalloc - device-synchronizing - than it saves. The accumulator is built on a thread of its own. It allocates and clears several hundred megabytes, and doing that in the constructor stalled the caller before it had read its first frame. Frames the device cannot take - anything but bitshuffle+LZ4 - still go to a host shard, so a run mixing compressions needs no second code path, and a build without CUDA is unchanged. RotationScaleMergeGPU set the CUDA device in its constructor and never put it back. CUDA's current device is per-thread, so that silently re-pinned the calling thread for the rest of its life, and the destructor freed several gigabytes against whatever device happened to be current by then - CudaDevicePtr records no device of its own. Every entry point now sets the device on entry and restores it on exit. ParallelFor/ParallelChunks moved to common/ParallelFor.h; two files had copies and a third wants them. Measured on a 16M-pixel rotation dataset: pre-scan 4.78 s -> 2.37 s -> ~2.0 s, shadow unchanged at 139126 pixels (22143 on a 2M-pixel dataset). Full 24-crystal battery: same space group on all 24, none failed, 15m32s -> 14m49s. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
20edf55063 |
Changelog: note the spot-finding, indexing and GPU memory gains
Build Packages / build:windows:nocuda (push) Successful in 12m53s
Build Packages / build:viewer-tgz:cpu (push) Successful in 18m50s
Build Packages / build:viewer-tgz:cuda (push) Successful in 21m40s
Build Packages / build:rpm (rocky9_nocuda) (push) Successful in 21m48s
Build Packages / build:rpm (ubuntu2404_nocuda) (push) Successful in 21m57s
Build Packages / build:rpm (rocky8_nocuda) (push) Successful in 25m22s
Build Packages / build:rpm (ubuntu2204_nocuda) (push) Successful in 25m22s
Build Packages / build:windows:cuda (push) Successful in 18m57s
Build Packages / build:rpm (ubuntu2204) (push) Failing after 13m29s
Build Packages / build:rpm (rocky9) (push) Failing after 16m57s
Build Packages / build:rpm (ubuntu2404) (push) Failing after 13m43s
Build Packages / build:rpm (rocky8) (push) Failing after 17m27s
Build Packages / build:rpm (rocky9_sls9) (push) Failing after 17m49s
Build Packages / build:rpm (rocky8_sls9) (push) Failing after 20m48s
Build Packages / Generate python client (push) Successful in 16s
Build Packages / Create release (push) Skipped
Build Packages / Build documentation (push) Successful in 43s
Build Packages / XDS test (neggia plugin) (push) Successful in 7m23s
Build Packages / XDS test (durin plugin) (push) Successful in 7m58s
Build Packages / XDS test (JFJoch plugin) (push) Successful in 7m56s
Build Packages / DIALS test (push) Successful in 12m37s
Build Packages / Unit tests (push) Successful in 1h17m37s
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VfYvJT5Nb71suJCowRBn5z |
||
|
|
312df30463 |
Score the first-pass validation frames together
Two-pass rotation indexing picks between candidate lattices by forcing each one and counting how many of 60 validation frames it indexes. That count was a serial loop, and it is the single largest serial stretch outside scale/merge: 57% of the 4.9 s first-pass phase, a few Ceres solves per frame on one core while the other 47 and all four GPUs sit idle. The frames are scored together now. Each one's verdict is its own - the score is only how many index - and this is the same call the main image loop already makes from every one of its workers on this same IndexAndRefine, which writes nothing but unit_cells[] under its own mutex. With the candidate forced, GetLattice() returns it and the branch that would advance the indexer's own state is never reached. The offline solver stops on an iteration count rather than a clock, so a loaded machine cannot change a frame's verdict. The spot cache had to be filled first: it is a plain map filled on demand, and a lookup racing an insert is not something a map survives. Filling it stays serial and in frame order, so its contents do not depend on scheduling. First-pass scheme gaps on the heaviest crystal 4.93 s -> 2.06 s, with both schemes returning the same 60/60 they did before. Battery 9m23s -> 9m01s, space group 21/24, no failures. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
|
83b33e19ee |
Give each FFT direction a block and its histogram shared memory
Cherry-picked from 2608-performance, restricted to the indexer: the same commit there also hoists a reciprocal out of a loop in the scaling code, which is not being touched on this branch. The histogram bins are unsigned integers and the counts stay below 2^24, so they convert to float exactly - the vote is bit-identical, and the indexing result with it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VfYvJT5Nb71suJCowRBn5z |
||
|
|
20ef59205b |
Build the GPU engines a worker never uses on first use, not always
Every worker thread built a full set of analysis engines. Two of them are never asked for on the offline path: the fixed-threshold spot finder, because detection is adaptive by default, and the azimuthal integrator, because the fused adaptive finder produces the profile as a by-product. They are still needed elsewhere - the broker defaults to non-adaptive detection, and --no-adaptive-spots asks for the finder - so they are built on first use rather than removed. A lazily built finder takes the current resolution mask on construction; without that it would find spots outside the limits it was never told about. The bitshuffle decoder sized its output buffer for the widest pixel type there is rather than the one the images actually have, holding a second full frame per worker on 16-bit data. It is sized from the image now and grows if a later frame needs more. The shared-table checksum runs over eight interleaved lanes. FNV's multiply is a loop-carried dependency, so one chain retires a byte every few cycles whatever memory bandwidth is spare, and every worker hashes tens of megabytes of geometry tables as it builds its engines - about 5% of all CPU samples on a 16M-pixel detector. Measured on a 16M-pixel rotation dataset: cudaMalloc 11314 -> 9474 calls and, with cudaFree, 117 s -> 78 s of aggregate thread time; both synchronise the whole device, so that time is spent blocking every other worker. Whole battery 15m32s -> 12m30s. Data quality against main, over 24 crystals and eight statistics each: the same space group on all 24, and every difference smaller than what two runs of an IDENTICAL binary produce (measured: 13 of 24 crystals reproduce exactly run to run, worst R_meas swing 5.5 points, against 4.6 points for main vs this branch). The float atomics in the reductions have always made this so. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
495e2d752d |
Read four pixels at a time in the ring reduction
reduce_rings_shared was 69% of all GPU kernel time - 116.8 s of a 70 s run across four cards. It is not bandwidth bound: flag_strong streams the same two arrays through the same grid-stride loop and reaches 196 GB/s, while this reached 30. The difference is the shared-memory atomics. Lanes in a warp read consecutive pixels along a detector row, a ring is a few pixels wide, so most of a warp lands in a handful of rings and the atomics to each one serialise. Two changes. The block reads four pixels per thread as one 16-byte and one 8-byte transaction, and merges the ones that fall in the same ring in registers before touching shared memory. Consecutive pixels usually DO share a ring, so this is where the win is: a run costs one set of atomics instead of one per pixel. npix is not guaranteed to be a multiple of four - it is width x height on the converted path, and detectors are not obliged to be even - so the vector loop stops short and a scalar loop finishes the remainder. Reading past the end would not fault, which is worse than if it did: it would fold uninitialised device memory into the accumulators and move the detection threshold in a way that does not reproduce. And the grid is sized from the occupancy the device reports, per pass. The two passes have different shared footprints - the first carries the corrected rings as well - so they do not fit the same number of blocks, and a grid sized for one left the other running a second wave at a quarter occupancy. The comment that justified the old grid reasoned from 1536 threads per SM, which is an Ada number; the card it ran on holds 1024. The run totals are still exactly what they were. The accumulators are unsigned 64-bit, so summing a run in a register and adding it once is the same value as adding each pixel separately - addition mod 2^64 is associative, overflow included - which is what keeps the ring statistics, and therefore the detection threshold, independent of how the work was grouped. That is the property the integer accumulators exist for. (The run accumulators are unsigned for the same reason: signed overflow would be undefined, and four squares of a large pixel value reach 2^64.) The corrected float sums, which feed the reported profile rather than any decision, change in their last bits as they already did between runs. Measured on a 16M-pixel rotation dataset: the kernel 116.8 s -> 19.1 s (6.1x), no longer the largest; the whole run 70 s -> 39.5 s. Full 24-crystal battery: same space group on all 24, none failed, 15m32s -> 12m47s. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
670831bad4 |
Stop allocating GPU and pinned memory nothing reads
Three resource fixes and two latent bugs, none of which changes a computed number. The preprocessed image has a host copy that only a CPU engine ever reads. On the GPU path every engine reads the device buffer instead, and rugnux always runs the fused adaptive finder, so that host copy is allocated, zeroed and PAGE-LOCKED for nothing - 72 MB per worker, 3.5 GB over 48 of them, and a cudaHostRegister each, which the driver serializes. It is now skipped by the same condition that already decides whether the device copies the image back. ImagePreprocessorBuffer keeps the pixel count separately so size() still answers when the mirror was not allocated. ROIIntegrationGPU asked device 0 for the SM count it sizes its grid from, while workers are pinned round-robin across the GPUs - so on a multi-GPU node it could size a grid from a card it never launches on. It asks the current device now, like every other engine. ~CudaRegisteredVector called a function that throws out of a destructor, and the move-assignment did the same from a noexcept function. Either would abort the process rather than report the failure, and teardown - after a device reset, or while another exception unwinds - is exactly where cudaHostUnregister fails. Both now use an unchecked unregister, as every other destructor in that header already does for its own teardown call. The throwing form stays for rebind()/unregister(), which are called from live code. Measured on a 16M-pixel rotation dataset: unchanged space group, merged reflection count and merging statistics. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
b74d8f8545 |
Give ParallelFor a home and a test
The two shapes - a fixed contiguous split, and work stealing off an atomic - had been copied into whichever file wanted them: an anonymous namespace in RotationScaleMerge.cpp, and another, byte for byte the same, in ShadowFinder.cpp, whose comment claimed it was "the only other file that wants it". The header is taken from the beam-stop GPU commit on the performance branch, which is not otherwise being picked. ShadowFinder now uses it, so the construct is exercised rather than shipped unused, and its own copy is gone. The beam-stop mask is unchanged, which its reference count already pins. Tested for the properties the callers rely on and which are easy to lose in a rewrite: the chunked slices tile the range in order with no empty one, work stealing visits every item exactly once, one thread means the caller's loop in order, an empty or negative count does nothing, an exception in a worker reaches the caller, and - the point of the whole thing - the answer is the serial answer bit for bit at every thread count. RotationScaleMerge.cpp keeps its own copy for now; consolidating it belongs with the scaling work, which is not being touched here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VfYvJT5Nb71suJCowRBn5z |
||
|
|
4e15fba98a |
Pin what the beam-stop parallelisation must not change
The mask rewrite replaces a BFS dilation with a separable box-max, a full-frame border flood with a bounded one, and three median passes with a single bin-and-rank - four separate equivalence arguments, none of them obvious by inspection. The reference count in the first test was taken from the serial implementation before any of it was picked, and is unchanged by all three commits. Two properties the reference alone cannot cover: the mask must not depend on how many threads split the per-pixel passes, and it must not depend on which shard a frame was accumulated into - including the maximum, which lives in a single shard when the reflection is on one frame. A four-armed scene is invariant under a quarter turn and so must its mask be, which is the sharpest probe available for the x and y passes being written differently. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VfYvJT5Nb71suJCowRBn5z |
||
|
|
7795ccb32b |
Give the process file its own thread
Writing an image to the process file takes the global HDF5 mutex, which is the same one every worker needs to find its next image. The write is short - the file holds the per-image analysis, not the pixels - but with a worker per hardware thread they were still taking turns at it. The workers now post to a bounded queue and one thread owns the file. A DataMessage does not own its pixels, it points into the reader's buffer, so the raw image is parked in the queue beside its message; without that the worker frees the pixels on its next iteration and the writer reads whatever landed there. The queue is bounded at four per worker so a run whose analysis outpaces its writer cannot accumulate every image it has ever processed, and a write that throws - out of space, above all - is held and rethrown when the loop drains it, before the end message is written and the file finalized. Worth 6.8 s -> 6.5 s on a 16 Mpx rotation dataset at 48 workers, on top of the much larger gain from taking the read out of the same lock. Both process files, written with and without the writer thread, re-scale to the same 101215 unique reflections at the same ISa. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
|
996cd20106 |
Make the beam-stop mask O(pixels) and parallel
GetMask() was 3.28 s of the 4.78 s pre-scan on a 16M-pixel detector, all on one thread. Four changes, none of which alters the mask: dilate() was a multi-source BFS. On a full rectangle with no obstacles the 8-connected graph distance IS the Chebyshev distance - a path stepping towards the target never has to leave the frame - so the result is a dilation by the (2r+1) square clipped to the frame, which separates into a pass along x and a pass along y. That is O(1) per pixel whatever r is, with no queue and no 4-bytes-per-pixel distance array (72 MB, allocated and filled five times per call). The erode() case is the one that hurt: it dilates the COMPLEMENT, so on a detector whose shadow is under 1% of the pixels it seeded the BFS from essentially every pixel. fill_holes() floods the background from the border. It now floods the bounding box of the region grown by one: everything outside that box is background and the box's own ring is background, so the whole outside is one border-connected component and a background pixel inside the box is border-connected exactly when it reaches the ring. The three baseline iterations re-binned every pixel by radius and re-took a median each time. The iteration only ever excludes pixels whose background is below a cut, and dividing by a positive baseline is monotone, so a ring's excluded pixels are exactly its lowest ones and the next median is an order statistic of the same, unchanging ring. The rings are binned and sorted once; each iteration then picks a rank and counts a prefix. Nine full-image passes become one. box_sum's vertical pass walked one column at a time, striding a whole row per step and missing on every access; it now carries a strip of columns together. Each row's and each column's running sum keeps its terms in its order, so the floating-point rounding is unchanged - only the traversal differs. The pooled COUNT is a count of at most 25 pixels, so it is an exact integer box sum now rather than a floating-point one; the background itself stays in double, because its running sum adds and subtracts across a whole row and in float the two roundings would not cancel. The per-pixel passes then run on all threads, and GetMask takes a thread count. Measured on a 16M-pixel rotation dataset: GetMask 3.28 s -> 0.99 s, whole pre-scan 4.78 s -> 2.37 s, whole run 1m10s -> 1m03s. The mask is unchanged on both a 16M and a 2M-pixel dataset (139126 and 22143 shadow pixels), as are the space group, the merged reflection count and the merging statistics. Also corrected the comment on erode(): the dilation cannot seed outside the frame, so outside behaves as foreground, not as complement. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
51c628af3b |
Parallelize the beam-stop pre-scan
The pre-scan read its sample of frames in a plain serial loop: one thread did the HDF5 read, the decompression and the full-detector accumulation for every frame. The cost is fixed per frame rather than per dataset, so it grew straight with detector area - measured at 0.9 s on a 2M-pixel detector and 7.9 s on a 16M-pixel one, where it was 11% of the whole run with 47 of 48 cores idle. Frames are now read on several workers. ShadowFinder keeps one projection per worker so nothing is locked while an image is added, and the projections are summed when the mask is read; the sums and counts are integers, so the result does not depend on how the frames were spread over the workers. A shard that never counted a pixel is skipped when the maxima are merged - it holds 0, which would otherwise beat a genuinely negative maximum. Worker count is capped (PRESCAN_MAX_WORKERS): a shard costs 20 bytes per pixel, and the accumulation is memory-bound, so a handful of workers already saturates it. The beam-centre spot pool is stitched together in sample order after the workers join, so frame numbering and the spot list are what the serial read produced regardless of how the workers interleaved. A frame still joins the pool only if it could be read. ShadowFinder::AddImage took its decompression scratch buffer BY VALUE, so the caller's buffer stayed empty and every frame allocated and zero-filled a fresh full-size uncompressed image (72 MB on a 16M-pixel detector) and freed it again. It takes a reference now, and each worker reuses one buffer. Measured on a 16M-pixel rotation dataset: pre-scan 7.9 s -> 4.8 s, whole run 69.2 s -> 65.1 s. Results are unchanged - same shadow pixel count, same space group, same merged reflection count and merging statistics on both a 16M and a 2M-pixel dataset, and the beam-centre path still commits the same centre. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
6190787913 |
Test that the beam-stop finder finds a beam stop
ShadowFinder has run by default in rugnux for a release and has never had a test. The scene is a beam stop - an opaque disk on the beam with an arm running off it to the edge - and the mask has to be that and nothing else: not the corners, and not a reflection recorded through the penumbra, which has to be given back. The last assertion pins the number of masked pixels as the serial implementation produces it. The detection is several passes of dilation, hole filling and a per-ring median, and a rewrite that moves the answer by a pixel would otherwise surface as a merging statistic several stages downstream, if at all. The scene is integer and noise-free so every mean is exact, and 257 is odd, square and not a multiple of 64 - the beam lands on a pixel, a cross is exactly 4-fold symmetric, and the column-blocked passes meet a short final block. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VfYvJT5Nb71suJCowRBn5z |
||
|
|
308aa1e0de |
Read raw images from several threads at once in a test
The three GetRawImage cases are single-threaded, so none of them enters the path the change is for: the chunk address is taken under the HDF5 lock and the bytes are read outside it, which only means anything when several workers are inside the reader at once - which is how rugnux drives it. Eight of them pulling every image and comparing against the bytes handed to the writer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VfYvJT5Nb71suJCowRBn5z |
||
|
|
92f2e0309e |
Keep the raw file alive while it is read without the lock
GetRawImage takes the chunk address under hdf5_mutex, drops the lock, and then reads through a borrowed RawFile*. ReadFile() and Close() both take that same lock and call Clear(), which empties the dataset cache and closes the descriptor - so a read racing a close read through a freed object and a recycled fd. Not reachable today, since the only callers of GetRawImage are the rugnux workers and jfjoch_extract_hkl and neither closes concurrently, but the whole point of the change is that the read happens outside the lock. Share the RawFile rather than borrowing it, so the descriptor outlives a Clear() that races it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VfYvJT5Nb71suJCowRBn5z |
||
|
|
c254081e58 |
Ask HDF5 where an image is, then read it without the lock
Two things every worker thread of an offline run did inside the global HDF5 mutex, per image. It opened /entry/data/data and asked it for its dataspace, its datatype and its creation plist, then asked those for the rank, the dimensions, the chunking and the compression. All of that is a property of the file and identical for all of its images, so it is now resolved once when the file is first touched. And it read the pixels - megabytes of them, with the lock held, which is what turned a worker per hardware thread into a queue. HDF5 can say where a chunk lives instead - address and byte count, a lookup in the chunk index with no read attached - so that is all it is asked for now, and the bytes are fetched after the lock is dropped, with a positional read that any number of threads can make through one handle at once. Chunk addresses count from the end of the user block, so its size is added; zero for anything this project writes, not for every file. A file that is not one chunk per image, or a chunk that was never written and exists only as a fill value, still goes the old way - only HDF5 knows what those read as. On a 16 Mpx rotation dataset with the process file being written, the per-image loop at 48 workers goes 12.4 s -> 6.8 s, and stops getting slower as workers are added: 8 workers were faster than 48 before, and are not now. Where no process file is written the same loop only improves ~1%, because this machine has 1.5 TB of RAM and held the whole 7 GB test set in page cache - the read was never the expensive part here. It is where the cache is cold or the filesystem is remote. Battery 9m45s, space group 21/24, no failures, unchanged. The Windows path uses ReadFile with an OVERLAPPED offset for the same reason pread is used elsewhere: it takes the offset as an argument rather than moving a shared file position, so the viewer keeps building under MSVC and gets the same concurrency. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
|
a1b48e9454 |
Mark unreadable frames in the reprocessing virtual dataset too
The master's own virtual dataset fills with the error marker, so a source file that cannot be resolved reads as masked rather than as zero counts. The virtual dataset rugnux writes into _process.h5 was left at HDF5's default fill of zero, which is a legitimate count - the same silent failure, one file along. The helper moves above its first user; it has to be set before SetVirtual. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VfYvJT5Nb71suJCowRBn5z |
||
|
|
b2058d1a79 |
Feed the ENOSPC test the pixel format it declares
DetJF4M is signed - a JUNGFRAU in photon-counting conversion is signed by default - and the fixture handed the writer uint16 images, so the pixel-format cross-check added in this branch refused them and the test failed before it reached what it is about. Nothing here reads a pixel value back. Missed when the other fixtures were corrected, because jfjoch_hdf5_enospc_test is a separate binary that jfjoch_test does not run; CI runs it as its own step. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VfYvJT5Nb71suJCowRBn5z |
||
|
|
d0ac559e64 |
Open a file whose sample axis does not turn
The reader looks for the goniometer by walking every leaf of /entry/sample/transformations and calling ReadAxis on each, stopping early only at an axis that is scanning. Not every leaf is an axis: the writer's own AXISNAME_end and the two rotation-width scalars carry units and nothing else, and ReadAxis threw when transformation_type was absent. A sweep survived on alphabetical order alone - omega sorts before omega_end, so the walk stopped before reaching it. A stationary axis never stopped, reached omega_end, and the open failed outright with "Cannot open attribute transformation_type". This branch is what made that reachable, by giving a still and a grid scan a spindle that stands still: rugnux read a grid-scan master, got a stationary axis, wrote _process.h5 through the goniometer path - which does emit omega_end - and could no longer open its own output. ReadAxis now treats a missing transformation_type as "not a transformation" and skips it, which is also what makes the search safe against anything a third party leaves in that group. JFJochReader_AxisRecovery covers what the reader has to recover: a sweep, a sweep about an axis that is not called omega, a spindle that does not turn, a grid scan alone and under a turning spindle, and a sweep with the head at a Smargon position. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VfYvJT5Nb71suJCowRBn5z |
||
|
|
d57f66a0b6 |
Docs: say what each downstream program actually does with our files
Build Packages / build:windows:nocuda (push) Successful in 11m38s
Build Packages / build:viewer-tgz:cpu (push) Successful in 19m39s
Build Packages / build:viewer-tgz:cuda (push) Successful in 22m23s
Build Packages / build:rpm (ubuntu2404_nocuda) (push) Successful in 23m23s
Build Packages / build:rpm (rocky9_nocuda) (push) Successful in 23m42s
Build Packages / build:rpm (rocky8_nocuda) (push) Successful in 28m23s
Build Packages / build:rpm (ubuntu2204_nocuda) (push) Successful in 28m35s
Build Packages / build:windows:cuda (push) Successful in 17m16s
Build Packages / build:rpm (rocky8_sls9) (push) Successful in 29m7s
Build Packages / build:rpm (rocky9_sls9) (push) Successful in 20m34s
Build Packages / XDS test (durin plugin) (push) Successful in 11m1s
Build Packages / build:rpm (rocky9) (push) Successful in 22m8s
Build Packages / Generate python client (push) Successful in 34s
Build Packages / Build documentation (push) Successful in 1m21s
Build Packages / Create release (push) Skipped
Build Packages / build:rpm (rocky8) (push) Successful in 27m33s
Build Packages / build:rpm (ubuntu2404) (push) Successful in 21m41s
Build Packages / build:rpm (ubuntu2204) (push) Successful in 26m32s
Build Packages / XDS test (neggia plugin) (push) Successful in 10m20s
Build Packages / XDS test (JFJoch plugin) (push) Successful in 11m1s
Build Packages / DIALS test (push) Successful in 23m15s
Build Packages / Unit tests (push) Failing after 1h51m32s
SOFTWARE_INTEGRATION.md said little more than which plugin to prefer. It now carries the layout matrix, since no program reads all three, and the things that silently give wrong answers rather than errors: - Neggia mis-reads signed 16-bit images. It dispatches on the pixel size in bytes and always casts to an unsigned type, so a count of -2 arrives as 65534 and the -32768 marker as 32768. JUNGFRAU in photon-counting conversion is signed by default, so this is the ordinary PSI case. Also noted in HDF5.md beside the fill-value description, where someone reading about the sentinel will meet it. - No XDS plugin reads saturation_value, so OVERLOAD has to be set by hand in XDS.INP. - XDS will not accept a negative MINIMUM_VALID_PIXEL_VALUE, so signed data cannot declare its negative counts valid at all. - The plugins act on different pixel_mask bits, so XDS and DIALS do not integrate the same pixels. - DIALS reads only the first data file of a multi-file NXmxLegacy set, and says nothing. - pyFAI learns no saturation value, marker or mask from a .poni and will integrate a sentinel as a count; the recipe given was checked against pyFAI's own NaN handling and matches it exactly. SECURITY.md was added to the tree but never to the toctree, so Read The Docs did not publish it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VfYvJT5Nb71suJCowRBn5z |
||
|
|
fae445c8ae |
Changelog: lead rc.162 with what it means for users
The block had grown into a list of field-level edits, several carrying rationale and measurements that belong in the commits. What a user needs from this release is one thing - files written by Jungfraujoch now import correctly in DIALS, XDS and pyFAI - so say that first and keep the rest to one line each. Also adds the security page, which shipped with no entry, and drops a test-only entry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VfYvJT5Nb71suJCowRBn5z |
||
|
|
eb0cb42355 |
Make the saturation limit convert once, in one place
The limit is EXCLUSIVE inside Jungfraujoch - the first value that is no longer a count - and NXmx saturation_value is INCLUSIVE, the highest value that still is one. XDS OVERLOAD and the DIALS trusted_range read it inclusively too. The write side subtracted the count and no read side added it back, so the value fell by one on every write-read-write cycle, unbounded: four chained runs over one dataset gave 32766, 32765, 32764, 32763. It also fed the preprocessor, so one more real count was called saturated after each cycle. SaturationValueFromLimit / SaturationLimitFromValue now carry the conversion, used by the writer and by all three readers (HDF5, the lite receiver, the viewer). JFJochReaderImage's summation test moves from > to >= in the same commit: it was silently compensating for the missing count, and correcting one without the other would have shifted it instead. Two more places said the wrong thing about the same pixels: error_value was GetUnderflow(), which is -1 for an unsigned image - a value no unsigned pixel can hold. The marker those images really carry is UINTx_MAX, and GetImageFillValue() already returned it, so the class held two disagreeing definitions of one marker. bit_depth_readout is now written for unsigned images only. DIALS remaps the top two codes of 2^bit_depth_readout to -1 and -2 whenever the field is present, without looking at the pixel type. For an unsigned image those fall below underload_value and are masked, which is what we want. For a signed one they land INSIDE the trusted range, so a saturated pixel reached DIALS as a trusted count of -2 - on the strongest reflections. Verified with DIALS 3.27: an int32 file now masks both sentinels. The field stays where it earns its keep, since dxtbx cannot read unsigned 32-bit without it. Neither the values themselves nor the wire format change. Verified against NXmx, DECTRIS SIMPLON, Durin (Global Phasing fork), XDS and DIALS 3.27; the chained run now holds at 32766. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VfYvJT5Nb71suJCowRBn5z |
||
|
|
583da3c6a0 |
Derive the rotation width for a chain that was sent
A chain carried in the END message was written verbatim and stopped there, so AXISNAME_end and the rotation width - which the writer produces when it builds the chain itself - were simply absent. A one-image sweep sent that way imported as a still, since dxtbx prefers AXISNAME_end and only falls back to np.diff; omega_range_average is what DECTRIS-oriented tooling reads for the oscillation. They are derived here rather than added to the wire format: for a constant step they follow from the values, which is every case there is today, so carrying them would cost an array per axis and say nothing new. The step is taken over the endpoints, because the values arrive as floats and a single difference puts that noise straight into the reported width. The test now compares the two routes on the files. It could not have caught this before: it went through the reader, and the reader reads neither of these. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VfYvJT5Nb71suJCowRBn5z |
||
|
|
c1b6030c4f |
Set the azimuthal reference in the .poni file
An image integrated in pyFAI through our .poni came out with every chi 180 degrees from where it belongs. pyFAI's in-plane axes are the negatives of ours, so Rot3 needs a half turn on top of the sign flip. Being a rotation about the beam it leaves 2theta alone - which is why radial integration was right all along and only the azimuth was wrong, and why a powder-ring check could never have caught it. The half turn is needed for the orientation-3 form written before rc.162 as well, so it is not an artefact of declaring the orientation - the file has been 180 degrees out for as long as it has been written. Verified against pyFAI 2026.5.0 on a tilted detector with an off-centre beam, against the lab positions of the NXmx chain: 2theta to 3.6e-15 deg and chi to 2.8e-14 deg. Then end to end, by integrating an image in jfjoch's own layout through a .poni the code actually writes: chi lands within 0.15 deg of physical truth on a 0.5 deg cake bin. Withdraws two changelog claims. The .poni does negate Rot3, and declaring orientation did not fix the azimuth: pyFAI's orientation is numerically inert here, so the file was relabelled and not corrected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VfYvJT5Nb71suJCowRBn5z |
||
|
|
ce11cade84 |
Tell a Smargon head position from the spindle by equipment_component
The reader recognised chi and phi by name. phi is an ordinary spindle name in MX, so a file whose rotation axis is called phi had it read back as a head position as well as the spindle - and writing that experiment out again threw, because the sample chain then tried to create phi twice. In the other direction a still with a head position had chi, its alphabetically first stationary axis, adopted as the goniometer. Both are now settled by the file: the axes jfjoch writes for a Smargon carry equipment_component="smargon", the reader takes a head position only from a tagged axis, and skips tagged axes when looking for the spindle. NXmx defines equipment_component as an identifier of the component of the equipment a transformation belongs to, which is what this is; there is no "equipment" attribute in NeXus at all. Adds HDF5Object::AttrExists, since the tag is absent on every file from anywhere else. The two tests assert on the written file - the axis length and the attribute - because the reader cannot see either: it does not look at a shape, and it did not look at the tag. That is the same gap that let the one-image shape through. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VfYvJT5Nb71suJCowRBn5z |
||
|
|
8dd3ee6576 |
Give the Smargon axes one entry per image
A reader takes the number of images from the innermost axis of the sample chain when no axis varies,
and chi/phi are innermost whenever they are present. Written as scalars, a still or a grid scan with
a recorded head position imported as ONE image however many were collected - dxtbx falls back to
nxsample.depends_on and takes num_images = len(scan_axis).
NXmx has no attribute that would say otherwise: there is no "equipment", and equipment_component
identifies a rigid assembly ("detector_arm", "detector_module"), which dxtbx reads only for the
detector module hierarchy. The axis length is what carries the image count.
Both writer paths are fixed, and the goniometer in BuildTransformationChain now takes its container
whenever the image count is known, as the writer already did - a stationary spindle sent over CBOR
had the same one-image shape.
Verified against DIALS 3.27: same file, chi/phi as scalars imports as 1 image, as per-image arrays
as 5.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VfYvJT5Nb71suJCowRBn5z
|
||
|
|
9ed299798d |
Stop tracking review notes and a generated version file
docs/review/ holds working material for a single task - review reports, work plans, investigation notes. It goes stale as soon as the code moves, and docs/conf.py has an empty exclude_patterns, so Sphinx would publish all of it. common/GitInfo.cpp is configured from GitInfo.cpp.in into the binary dir; the tracked copy was the residue of an in-source configure and still named rc.148. Neither was ever meant to be committed - both arrived through a git add -A. Both are now gitignored, and CLAUDE.md says to stage by explicit path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VfYvJT5Nb71suJCowRBn5z |
||
|
|
e4edcd6fa9 |
Carry the sample transformation chain as DetectorTransformation
Replaces the start-message TransformationAxis of the previous commit, which was the wrong shape in two ways. DetectorTransformation (common/) mirrors a NeXus NXtransformations axis and holds nothing else: name, type, units, vector, offset, depends_on and the positions themselves. Deliberately without cleverness - the values are either a single number for an axis that does not move or one per image, and nothing derives a position from a start and an increment. That is the point: a producer will later want to report where a stage actually WENT rather than where it was told to go, and a structure that stores start+increment cannot express that. A million images cost 4 MB per axis, which is not a reason to be clever. Hence also the move to the END message: measured positions are only known once the run is over. And hence no metadata version bump, which the previous commit did make. The chain is optional; when it is absent the writer builds the identical chain from the start message, exactly as before. Nothing on the wire changes for a producer that does not send it, so a broker and a writer of different releases still interwork - the constraint the previous version stated is withdrawn. The writer transcribes a chain it is given, without recomputing an angle, which is what makes measured positions possible end to end. JFJochReader_TransformationChain_SentAndBuilt writes the same run both ways and checks the two files read back the same, chi/phi included. CBORSerialize_End_Transformations covers the wire 1:1, asserting the order survives and that a moving axis keeps one value per image while a stationary one keeps a single value. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
7efcf631de |
Write module_offset as a float, and declare offset_units
Build Packages / build:viewer-tgz:cpu (push) Successful in 13m34s
Build Packages / build:viewer-tgz:cuda (push) Successful in 15m43s
Build Packages / build:rpm (rocky9_nocuda) (push) Successful in 19m43s
Build Packages / build:rpm (rocky8_nocuda) (push) Successful in 23m14s
Build Packages / build:rpm (ubuntu2404_nocuda) (push) Successful in 18m44s
Build Packages / build:rpm (ubuntu2204_nocuda) (push) Successful in 24m21s
Build Packages / build:rpm (rocky9_sls9) (push) Successful in 18m50s
Build Packages / build:rpm (rocky8_sls9) (push) Successful in 22m37s
Build Packages / build:rpm (rocky9) (push) Successful in 19m37s
Build Packages / XDS test (durin plugin) (push) Successful in 11m11s
Build Packages / build:rpm (ubuntu2404) (push) Successful in 19m20s
Build Packages / build:rpm (rocky8) (push) Successful in 26m9s
Build Packages / Generate python client (push) Successful in 39s
Build Packages / Create release (push) Skipped
Build Packages / Build documentation (push) Successful in 1m13s
Build Packages / build:rpm (ubuntu2204) (push) Successful in 24m8s
Build Packages / XDS test (JFJoch plugin) (push) Successful in 9m22s
Build Packages / XDS test (neggia plugin) (push) Successful in 7m20s
Build Packages / DIALS test (push) Successful in 19m31s
Build Packages / Unit tests (push) Failing after 1h22m37s
Build Packages / build:windows:nocuda (push) Successful in 11m9s
Build Packages / build:windows:cuda (push) Successful in 13m51s
Two latent traps in the NXtransformations attributes, both inert today and both wrong the moment they are not. module_offset was an int32 with @vector = (0,0,0). NXmx types the field NX_FLOAT, and a translation needs a unit vector to be well formed - the direction of a zero-magnitude translation is arbitrary, not absent. Now a float with (0,0,1); the transformation it describes is unchanged, since the magnitude is still zero. Every transformation that carries an @offset wrote it without @offset_units, and every caller passed an empty string. A reader then falls back to the axis's own `units` - which on a rotation axis is degrees - and converts a length from degrees to millimetres. nxmx only performs that conversion when the offset is non-zero, so ours have never triggered it, but the first non-zero offset on a goniometer axis would. The helper now always declares it, rather than leaving it to a caller to remember. Measured after the change: module_offset is H5T_IEEE_F32LE, a rotation axis carries offset_units "m" beside units "deg", dials.import still reads the file, and the tilted-geometry cross-check is unchanged at 1.6e-6 mm. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
842c43a86e |
Send the sample transformation chain in mounting order
Build Packages / build:windows:nocuda (push) Successful in 2m34s
Build Packages / build:viewer-tgz:cpu (push) Successful in 17m9s
Build Packages / build:viewer-tgz:cuda (push) Successful in 19m41s
Build Packages / build:rpm (rocky9_nocuda) (push) Successful in 22m39s
Build Packages / build:rpm (ubuntu2404_nocuda) (push) Successful in 23m20s
Build Packages / build:rpm (rocky8_nocuda) (push) Successful in 27m40s
Build Packages / build:rpm (rocky8_sls9) (push) Successful in 28m55s
Build Packages / build:rpm (ubuntu2204_nocuda) (push) Successful in 29m34s
Build Packages / build:rpm (rocky9_sls9) (push) Successful in 19m34s
Build Packages / XDS test (durin plugin) (push) Successful in 12m14s
Build Packages / build:rpm (rocky9) (push) Successful in 22m26s
Build Packages / Generate python client (push) Successful in 41s
Build Packages / build:rpm (rocky8) (push) Successful in 27m9s
Build Packages / Create release (push) Skipped
Build Packages / Build documentation (push) Successful in 1m15s
Build Packages / XDS test (JFJoch plugin) (push) Successful in 11m36s
Build Packages / build:rpm (ubuntu2404) (push) Successful in 20m45s
Build Packages / build:rpm (ubuntu2204) (push) Successful in 24m42s
Build Packages / XDS test (neggia plugin) (push) Successful in 8m29s
Build Packages / DIALS test (push) Successful in 23m27s
Build Packages / Unit tests (push) Failing after 1h54m37s
Build Packages / build:windows:cuda (push) Successful in 18m2s
The chain could not be expressed. A goniometer axis, the Smargon chi/phi and a grid stage each travelled by a different route - the `goniometer` map, a private JSON key inside user_data, and `grid_scan` - and nothing said what order they are mounted in. The order cannot go in the `goniometer` map either: that is a DECTRIS stream2 field, and RFC 8949 requires deterministic encoders to SORT map keys, so a map's order is not something a consumer may rely on. So `transformations` is sent as an ordered ARRAY, base first, each element carrying name, type, axis vector and angles. The `goniometer` map and `grid_scan` are still emitted beside it, unchanged, for consumers that only know stream2 - nothing vendor-defined is mutated, and a stream2 consumer sees exactly what it saw before. Smargon chi and phi are ordinary stationary axes that were only ever separate for historical reasons, and they now appear in the chain like any other. They are also read back: reader/ had no smargon support at all, so re-opening a file lost the head position silently. Combined with the earlier change that writes them for a still rather than only alongside a rotation or a grid scan, the round trip is now closed. Metadata version 7. A broker and a writer from different releases must not be mixed across this: an older writer ignores the chain and reads the unordered map, so anything whose order matters - a Smargon position, or a grid scan combined with a rotation - is not reproduced. Said so in docs/CBOR.md and the changelog. CBORSerialize_Start_Transformations asserts the ORDER survives, not just the contents, which is the whole point of the array. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
d2f57975e8 |
Record whether the image is mirrored in Y, and label the .poni orientation
Which way the detector's rows run was decided once, in the module assembly, and never stated again: not on the wire, not in the file, nowhere a consumer could read it. mirror_y was consumed inside the DetectorGeometryModular constructor and discarded. It is now a declared property of the detector setup, carried into the start message, written to HDF5 under detectorSpecific, and read back. Absence means true, which is the MX convention and the only thing Jungfraujoch has ever produced. Deliberately a boolean and not a corner enum: the assembled image can only be flipped in Y, so a four-corner value would encode states that cannot occur. DECTRIS stream2 has no field for this - checked against the specification - so the key is new rather than an extension of theirs, and a consumer that does not know it skips it and behaves exactly as before. The .poni file gains pyFAI's orientation. Without it pyFAI applies its own default, 3 (bottom left), and believes increasing row means physically upwards. The numbers still agreed - a mirror preserves 2theta, so radial integration was never affected - but the azimuth came out with the opposite sense, which matters for cake and sector integration. Declaring orientation 2 is not a one-line addition: it re-anchors Poni1 to the top edge and reverses rot2 and rot3, a row flip being improper. Measured against pyFAI 2026.5.0 by searching all four orientations, both Poni1 anchorings and all eight sign combinations: exactly two combinations reproduce the lab position DiffractionGeometry computes to 1.4e-17 m - the unlabelled form written before, and (orientation 2, Poni1 = height-1-beam_y, +rot1/+rot2/-rot3), which is now written. Calibration_PoniFileAxisConvention pins it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
5e05515165 |
Write the grid stage as a base stage, not head-mounted
The grid translations went innermost, i.e. mounted on the head, so a grid position turned with the spindle. At SLS the grid is an Aerotech xyz that the spindle is mounted ON, so the mounting order is base -> grid -> omega -> chi -> phi -> sample and a grid position is independent of omega. Identical to the previous chain at omega = 0, which is every grid scan collected so far, and correct rather than incorrect when it is not. A head-mounted stage exists too - the Smargon translates, and that is what helical uses - and would sit on the other side of omega. Only the base stage is modelled for now, which is the one actually used; the comment says so. Measured: dials.import reads a master with the new chain. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
cc334c5c55 |
Name the right rotation in the NXmx frame comment
The comment said internal-to-McStas is 180 degrees about x. It is 180 degrees about z: internal (a,b,c) maps to McStas (-a,-b,+c), which is what makes the written vectors correct - internal +y becomes (0,-1,0), the -x of Rx(-rot2) becomes (1,0,0), and the -z of Rz(-rot3) is unchanged. 180 degrees about x is the internal-to-imgCIF relation, one step further on, and it is the frame the verification was done in - which is why the vectors are right and only the prose was wrong. Re-measured after the change: still 1.6e-6 mm over nine tilt settings. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
7039aa4e45 |
Stop treating a goniometer axis and a grid scan as alternatives
They are not alternatives: a grid is usually collected at a particular head position, so an axis and a grid describe different parts of the same setup. The exclusion was enforced independently in four places - the API converter, the CBOR serializer, the writer and the reader - and each silently dropped the grid scan when an axis was present. Nothing warned. The writer now builds one chain from the base outwards, spindle -> chi -> phi -> helical -> grid translations, instead of two branches. NXmx applies the deepest dependency first, so the sample ends up innermost, which is what it physically is: the grid stage rides on the head and the head rides on the spindle. The grid translations consequently move inside the rotation - identical to before at omega = 0, and right rather than wrong when it is not. A grid scan with no axis at all now writes a stationary omega. NXmx has no way to say "there is no rotation", and a sample chain of translations alone is not something readers accept: dxtbx raises outright on it, so every grid-scan master we have written so far cannot be opened by DIALS. Measured on a file matching the new chain: dials.import reads it. At 0 degrees the rotation is the identity whatever the axis points along, so the conventional vector carries no geometric claim - it only has to be well formed. The API change is deliberately not breaking: no field changes type or cardinality, only the prose saying the two were exclusive, and a request that set both used to lose one silently and now does not. JFJochReader_GridScan asserted the absence of a goniometer; it now asserts the axis is present and stationary, which is the contract that matters - a grid scan must not read back as a sweep. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
b35672a0c3 |
Read any rotation axis by name, and tell a stationary axis from a sweep
Two things the goniometer handling conflated. The axis name is free-form everywhere that writes it - the API imposes only minLength, the CBOR map uses the name as its key, and tests/CBORTest.cpp round trips one literally called "z" - but the reader looked for exactly "/entry/sample/transformations/omega". A sweep recorded as "phi" therefore came back as stills, in the viewer and in rugnux, with nothing to indicate it. The reader now walks the transformations group and takes whichever axis is a rotation, preferring one that turns; the grid scan is read independently rather than as the else-branch of the same test, since a grid scan can be taken at a given head position. Second: "an axis is defined" and "the axis is turning" were the same question, answered inconsistently - GetImagesPerFile checked the increment, IsRotationIndexing did not, and the CBOR decoder deleted zero-increment axes outright so the ambiguity could never surface. GoniometerAxis::IsScanning now asks it explicitly and the call sites go through it, so a stationary axis can be carried without being mistaken for rotation data. That mistake is not hypothetical: RotationIndexerCounter leaves its stride at zero for a zero increment, and Process() then never fires, so indexing would silently never run. Keeping stationary axes is also what lets the writer state where the head was for a still or a grid scan, which is the next step. JFJochReader_Goniometer_NonOmegaName covers the naming case through the writer and back; nothing did before, because both existing round trips use "omega". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
1f4f77fe42 |
Choose images_per_file from the acquisition when it is not given
The value means three different things at once. It is the unit of writer parallelism - whole files go round-robin to the writers, (image_number / images_per_file) % socket.size() in ZMQStream2Pusher::SendImage and TCPStreamPusher - it multiplies writer memory linearly, since every data-file plugin reserves per file, and it decides whether a legacy master is readable at all, because dxtbx follows only the first data file of one. That last point is what makes a flat default wrong. Measured with DIALS on a 2500-image rotation sweep written as legacy: split into five files it reports 2500 images and then raises IndexError beyond image 499, so it half-works silently; in one file all 2500 read. AutoPROC does not read VDS, so legacy has to stay the default, which leaves the file count as the only lever. So make it optional and resolve it from the acquisition. A rotation sweep of at most 20000 images goes into one data file - rotation datasets are small enough, and one writer keeps up with them. A grid scan splits on whole fast-axis rows, so a file is a meaningful piece of the grid. Stills and serial keep 1000, where the image count far exceeds it and the parallelism and the bounded writer memory are what matter. An explicit value is always taken literally. GetImagesPerFile is the single place this is resolved, and it must always return a fixed non-zero number, because everything downstream - receiver, pusher, puller, writer - requires one. That was already true of the old 0 = "one file" spelling; 0 is now gone from the API and omitting the field says the same thing better. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
259b43154e |
Writer: refuse a mistyped stream, and mark unreadable VDS frames
Two ways the written files could misdescribe themselves without anyone noticing. The pixel format is stated twice and the two were never compared: each image carries its own type as a CBOR tag, which is what the data files are written with, while the master is typed from the start message. A stream whose header contradicts its images produced data files of one type under a master declaring another, and with NXmxVDS, HDF5 then converts silently on every read. HDF5DataFile::CreateFile now checks the two agree and refuses the run otherwise - the point where the values first meet, so it covers every path into the writer. Two test fixtures were relying on exactly that inconsistency. The HDF5 writer tests wrote uint16 buffers under a JUNGFRAU experiment, which converts to photon counts by default and so declares int16; they never read the pixels back, so it went unnoticed. The receiver-lite tests feed frames from compression_benchmark.h5, which really are signed int16, through a DECTRIS experiment, which declares unsigned by default - the same class of bug the pixel_signed propagation fixed on the live path. Both now declare what they send. Second: a virtual dataset whose source file is absent reads as the fill value, and HDF5 defaults that to zero, so a data file that was not copied alongside the master is indistinguishable from frames of genuine zero counts. Measured with DIALS on a four-file set with one file removed: 25 frames of pure zeros, no error and no warning. The image VDS is now filled with the error marker instead, which sits outside underload_value..saturation_value, so a reader masks those frames. Same measurement after the change: -32768 throughout, which DIALS excludes. Only the images ask for a fill value; the per-image metadata datasets keep the default. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
28cb185325 |
rugnux: log the detector geometry in XDS's convention
Build Packages / build:windows:nocuda (push) Successful in 13m32s
Build Packages / build:windows:cuda (push) Successful in 19m31s
Build Packages / build:viewer-tgz:cpu (push) Successful in 14m55s
Build Packages / build:viewer-tgz:cuda (push) Successful in 15m55s
Build Packages / build:rpm (rocky9_nocuda) (push) Successful in 16m59s
Build Packages / build:rpm (rocky8_nocuda) (push) Successful in 19m21s
Build Packages / build:rpm (ubuntu2404_nocuda) (push) Successful in 15m57s
Build Packages / build:rpm (ubuntu2204_nocuda) (push) Successful in 20m48s
Build Packages / build:rpm (rocky8_sls9) (push) Successful in 21m1s
Build Packages / build:rpm (rocky9_sls9) (push) Successful in 18m38s
Build Packages / build:rpm (rocky8) (push) Successful in 20m31s
Build Packages / build:rpm (rocky9) (push) Successful in 18m45s
Build Packages / XDS test (durin plugin) (push) Successful in 9m43s
Build Packages / build:rpm (ubuntu2204) (push) Successful in 21m22s
Build Packages / Generate python client (push) Successful in 13s
Build Packages / build:rpm (ubuntu2404) (push) Successful in 15m16s
Build Packages / Create release (push) Skipped
Build Packages / Build documentation (push) Successful in 51s
Build Packages / XDS test (JFJoch plugin) (push) Successful in 7m17s
Build Packages / DIALS test (push) Successful in 17m34s
Build Packages / XDS test (neggia plugin) (push) Successful in 6m10s
Build Packages / Unit tests (push) Successful in 1h22m42s
XDS is never handed this geometry - the durin plugin gives it image data only
(plugin_get_header returns dimensions, bytes per pixel, pixel size and frame
count, nothing more) and XDS refines its own from XDS.INP. That is exactly what
makes printing ours in the same convention useful: it turns "does our geometry
agree with XDS's refinement" into reading two logs side by side.
The two laboratory frames already coincide - x along increasing detector column,
y along increasing row, z along the beam - so nothing is converted. XDS places a
pixel at
x_lab(i,j) = (i-ORGX)*QX*X_axis + (j-ORGY)*QY*Y_axis + DISTANCE*(X_axis x Y_axis)
which is DiffractionGeometry::LabCoord with X_axis = poni_rot*(1,0,0) and
Y_axis = poni_rot*(0,1,0). The axes are taken as differences of LabCoord so they
track whatever the geometry currently is, tilt included, and a tilt goes out as
the two axis vectors rather than as angles - the form XDS itself reports after
refinement.
Two traps are handled and documented: ORGX/ORGY are 1-based, XDS counting pixels
from 1 where we count from 0; and they are the PONI, the foot of the
perpendicular from the crystal, which is what our beam centre is too but is not
the direct beam once the detector is tilted.
ROTATION_AXIS is printed as stored. The direction is right, the frames being
shared, but its sign has not been cross-checked against an XDS refinement, so a
flip there should be read as unconfirmed rather than as a real disagreement.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
659ba0d5b9 |
Cross-check a tilted detector against pyFAI and DIALS
Nothing constrained the tilted geometry. Every existing test is either self-consistent or moves one angle at a time, and every file CI writes has zero tilt - where a swapped axis, a reversed composition order and a wrong pivot all give exactly the same answer. Two real bugs lived in that gap. Two checks, because there are two things to guard. DiffractionGeometry_Tilted_vs_PyFAI_and_DIALS pins the model itself: all three PONI angles non-zero and of mixed sign, compared per pixel against reference positions from pyFAI (an independent implementation of the convention) and from DIALS, to 2 um. Because the references are quoted in their own frames and the test applies the documented mappings - pyFAI (t1,t2,t3) -> our (x,y,z), and imgCIF = ours turned 180 degrees about x - it pins those relations too, not just the arithmetic. The comment gives the snippets to regenerate both sets. tests/nxmx_geometry_dials_test.py guards the writer, which is where the bugs actually were and which the unit test cannot reach. CI writes a master, patches the geometry in and asks DIALS where the panel is. Only the angle VALUES are patched; the axis vectors, the depends_on chain and the pivot stay as the writer emitted them, so they remain under test. Verified to fail on the pre-fix encoding: 7.3 mm, exit 1, naming the chain as the thing to look at. Recorded in both, because it cost an hour: compare via get_origin() and the fast/slow axes, NOT get_pixel_lab_coord(), which applies a parallax correction from the sensor thickness that Jungfraujoch does not model - about 0.1 mm at the detector edge, easily mistaken for a geometry error. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
1eefd035c2 |
rugnux: stop negating Rot3 in the .poni file
Build Packages / Unit tests (push) Successful in 1h53m42s
Build Packages / build:windows:nocuda (push) Successful in 14m4s
Build Packages / build:windows:cuda (push) Successful in 21m25s
Build Packages / build:viewer-tgz:cpu (push) Successful in 11m53s
Build Packages / build:viewer-tgz:cuda (push) Successful in 15m2s
Build Packages / build:rpm (rocky8_nocuda) (push) Successful in 20m56s
Build Packages / build:rpm (rocky9_nocuda) (push) Successful in 18m47s
Build Packages / build:rpm (ubuntu2204_nocuda) (push) Successful in 20m21s
Build Packages / build:rpm (ubuntu2404_nocuda) (push) Successful in 15m15s
Build Packages / build:rpm (rocky8_sls9) (push) Successful in 18m27s
Build Packages / build:rpm (ubuntu2204) (push) Successful in 18m53s
Build Packages / build:rpm (ubuntu2404) (push) Successful in 13m50s
Build Packages / DIALS test (push) Successful in 20m13s
Build Packages / XDS test (durin plugin) (push) Successful in 8m39s
Build Packages / XDS test (JFJoch plugin) (push) Successful in 8m59s
Build Packages / XDS test (neggia plugin) (push) Successful in 9m2s
Build Packages / Generate python client (push) Successful in 29s
Build Packages / Build documentation (push) Successful in 1m21s
Build Packages / Create release (push) Skipped
Build Packages / build:rpm (rocky9) (push) Successful in 14m1s
Build Packages / build:rpm (rocky9_sls9) (push) Successful in 14m11s
Build Packages / build:rpm (rocky8) (push) Successful in 14m30s
The PONI export mapped the internal angles to pyFAI as (+rot1, -rot2, -rot3). Checked against pyFAI 2026.5.0 directly - building a Geometry from the exported values and comparing calc_pos_zyx against the lab position DiffractionGeometry computes, per pixel over the whole detector and with each angle exercised on its own - the correct mapping is (+rot1, -rot2, +rot3): it agrees to 1.4e-17 m, while negating rot3 puts a pixel 25 mm out on a rot3-only geometry. rot1 and rot2 were already right, which is consistent with how this was originally validated: a LaB6 powder image, where rings sharpened once the rot2 flip was applied. That check could not have caught rot3, because a rotation about the beam leaves q and 2theta invariant and moves only the azimuth - so the error only ever showed up in cake/sector integration, and only for a detector actually rotated about the beam. rot3 is never refined and has no CLI flag, so in practice it is almost always zero. The old comment derived the signs from "a reflection in y between the MX and pyFAI frames". That gives the right answer for rot1 and rot2 and the wrong one for rot3, and pyFAI's own documentation contradicts itself on the direction of its axis 2, so the comment now records the empirical pin instead of a derivation. Calibration_PoniFileAxisConvention previously set rot3 to zero and asserted Rot3 == 0.0 - the one cell that could not fail. It now uses a non-zero rot3. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
03e481ff2a |
Export the detector tilt correctly in the NXmx transformation chain
Build Packages / build:windows:nocuda (push) Successful in 11m32s
Build Packages / build:windows:cuda (push) Successful in 14m21s
Build Packages / build:viewer-tgz:cpu (push) Successful in 12m32s
Build Packages / build:viewer-tgz:cuda (push) Successful in 14m42s
Build Packages / build:rpm (rocky9_nocuda) (push) Successful in 17m0s
Build Packages / build:rpm (rocky8_nocuda) (push) Successful in 19m48s
Build Packages / build:rpm (ubuntu2204_nocuda) (push) Successful in 19m48s
Build Packages / build:rpm (ubuntu2404_nocuda) (push) Successful in 14m25s
Build Packages / build:rpm (rocky8_sls9) (push) Successful in 19m56s
Build Packages / build:rpm (rocky9_sls9) (push) Successful in 18m14s
Build Packages / build:rpm (rocky8) (push) Successful in 21m13s
Build Packages / build:rpm (rocky9) (push) Successful in 18m51s
Build Packages / build:rpm (ubuntu2204) (push) Successful in 20m20s
Build Packages / XDS test (durin plugin) (push) Successful in 8m53s
Build Packages / build:rpm (ubuntu2404) (push) Successful in 16m22s
Build Packages / Generate python client (push) Successful in 19s
Build Packages / Create release (push) Skipped
Build Packages / Build documentation (push) Successful in 1m1s
Build Packages / XDS test (JFJoch plugin) (push) Successful in 8m46s
Build Packages / XDS test (neggia plugin) (push) Successful in 7m6s
Build Packages / DIALS test (push) Successful in 16m54s
Build Packages / Unit tests (push) Successful in 2h37m13s
Three independent errors, all invisible while the tilt is zero - which it is in every test and every CI file, and which is why this survived. 1. rot1 and rot2 carried each other's axis. Jungfraujoch holds the tilt in the PyFAI PONI convention, poni_rot = Rz(-rot3)*Rx(-rot2)*Ry(+rot1), written in the internal frame (x along increasing column, y along increasing row, z along the beam). NXmx uses McStas, which is that frame turned 180 degrees about x - a proper rotation, NOT a mirror - so rotations about y and z reverse sense and those about x do not. Correct vectors are rot1 (0,-1,0), rot2 (1,0,0), rot3 (0,0,-1); only rot3 was already right. 2. The depends_on chain composed the rotations in the reverse order. A chain applies the deepest dependency first, so rot3 has to sit at the root for the product to be R_rot3*R_rot2*R_rot1. Second-order: it only shows up when two angles are non-zero at once. 3. The tilt pivoted about the wrong point. With translation at the root, a reader takes the panel origin as the unrotated vector and merely reorients the panel, while Jungfraujoch rotates the whole sample->pixel vector including the distance. Moving translation inside the rotations fixes the pivot; this was the largest of the three and is invisible to any test that only checks axes. Verified against DIALS 3.27 by comparing the lab position of nine pixels spread over the detector against poni_rot applied to the sample->pixel vector, over nine tilt settings including combined and mixed-sign angles: max error 1.6e-6 mm, which is float32 rounding of the stored metadata. The shipped encoding gives 40.7 mm - 542 pixels - at a (0.2, 0.3, 0.15) rad tilt. Note when comparing by hand that dxtbx applies a parallax correction in its pixel->mm conversion, which Jungfraujoch does not model; get_pixel_lab_coord is therefore not the right comparison point and shows a ~0.1 mm radius-dependent offset that is not a geometry error. Jungfraujoch's own reader is unaffected: it reads rot1/rot2/rot3 by dataset path and never consults the vectors or the chain, which is why the round trip stayed self-consistent throughout. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
fccf2d911a |
VERSION: 1.0.0-rc.162
Build Packages / build:windows:nocuda (push) Successful in 12m11s
Build Packages / build:windows:cuda (push) Successful in 15m16s
Build Packages / build:viewer-tgz:cpu (push) Successful in 17m24s
Build Packages / build:viewer-tgz:cuda (push) Successful in 18m18s
Build Packages / build:rpm (ubuntu2404_nocuda) (push) Successful in 19m9s
Build Packages / build:rpm (rocky9_nocuda) (push) Successful in 20m59s
Build Packages / build:rpm (rocky8_nocuda) (push) Successful in 23m55s
Build Packages / build:rpm (ubuntu2204_nocuda) (push) Successful in 23m52s
Build Packages / build:rpm (rocky9_sls9) (push) Successful in 18m3s
Build Packages / build:rpm (rocky8_sls9) (push) Successful in 23m35s
Build Packages / build:rpm (rocky9) (push) Successful in 20m12s
Build Packages / build:rpm (ubuntu2404) (push) Successful in 21m11s
Build Packages / build:rpm (rocky8) (push) Successful in 25m40s
Build Packages / Generate python client (push) Successful in 28s
Build Packages / Build documentation (push) Successful in 1m7s
Build Packages / Create release (push) Skipped
Build Packages / build:rpm (ubuntu2204) (push) Successful in 26m2s
Build Packages / XDS test (durin plugin) (push) Successful in 9m13s
Build Packages / XDS test (JFJoch plugin) (push) Successful in 9m40s
Build Packages / XDS test (neggia plugin) (push) Successful in 7m42s
Build Packages / DIALS test (push) Successful in 19m36s
Build Packages / Unit tests (push) Successful in 2h21m23s
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
2c202dafee |
Docs: tighten rc.162 changelog entries and note what _process.h5 links to
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
f2a780cdb5 |
CI: check the image count in the DIALS test, and fix the integrated-format filename
Three problems with the DIALS job. It asserted nothing but xia2.ssx's exit code, and dxtbx reports a short scan rather than failing when it cannot reach every image - xia2.ssx then succeeds on the images it did get, so silent truncation looked like a pass. Add dials.import with an explicit "num images: 100" check. The single-file step ran `xia2.ssx image=single.h5`, but `-S -o single` writes single_master.h5. The file never existed, so NXmxIntegrated was not actually being tested by DIALS at all. Multi-file NXmxLegacy stays out of this job on purpose, with a comment saying why: dxtbx takes one dataset out of NXdata and indexes it globally, so a legacy master reads back as only its first data file (measured: 40 images written, 10 imported, no warning). The XDS jobs below do build that layout and read it correctly, which is why the gap survived - the layout and DIALS were each covered, never together. Verified locally against DIALS 3.27: all three formats now report 100 images. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
d01e69e407 |
rugnux: describe the linked images in _process.h5, not the processing container
_process.h5 is an NXmxIntegrated master that links to the ORIGINAL image files rather than writing images of its own (write_images = false). Its pixel metadata therefore has to describe those files - but it was filled from experiment_, which Rugnux pins to signed 32-bit because that is the container HDF5MetadataSource hands images out in. The virtual dataset was consequently typed int32 over unsigned 16- or 32-bit sources, so HDF5 converted every value on read: the 0xFFFFFFFF error marker of a uint32 source does not survive, and error_value and underload_value described a container the file does not contain. Our own reader never saw it, because it resolves the mapping and opens the source file itself. Only consumers that go through the virtual view - DIALS, XDS via Durin, plain h5py - read the converted values. Take the format from the reader instead. GetStoredPixelFormat() reports the bit depth and signedness of /entry/data/data as stored, which is deliberately not the same thing as the experiment's image format, and rugnux fills the start message from it. The writer is left alone on purpose: it must be able to produce a master before the mapped files exist, or when they are not readable, and the mapping strips the source directory, so it cannot open them to ask. Measured on a 20-image set with int16 sources: the _process.h5 virtual dataset goes from H5T_STD_I32LE to H5T_STD_I16LE, matching the source, and DIALS reports trusted_range (-32767, 32765) instead of (-2147483647, 32765). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
77c6d0f4ec |
Report the stored image depth in bit_depth_readout, and write underload_value
NXmx has no field for the depth of the stored image - only bit_depth_readout, "how many bits the electronics record per pixel". The two diverge exactly when summation is used: the readout keeps the detector's native width while the summed image must be wider to hold the sum. Every NXmx reader nonetheless takes bit_depth_readout as the width of the stored pixel. dxtbx ignores the non-standard bit_depth_image entirely for a generic NXmx file, derives its masking markers from bit_depth_readout, and raises "Unsupported integer dtype uint32" for a 32-bit image when the field is absent. Reporting the electronic value there would mislead precisely where it differs. So report the image depth in both fields, and drop the machinery that existed to carry the electronic one for a DECTRIS detector: the SIMPLON read, the DetectorSetup setter, and the receiver-side propagation of a key that the DECTRIS stream2 protocol does not even define. JUNGFRAU and PSI EIGER keep their readout depth, which the FPGA acquisition genuinely needs. Also write NXmx underload_value, the lowest valid value. Without it a reader takes the trusted minimum to be -0x7FFFFFFF, so the error-pixel marker sits inside the trusted range and is consumed as an intensity. Measured with DIALS 3.27 on a written file: trusted_range goes from (-2147483647, 32766) to (-32767, 32766), so the INT16_MIN gap pixels are now masked. Third fix in the same area: JFJochReceiverLite::Configure took the image width from the incoming stream but not the sign, while the image itself is forwarded byte-for-byte. A detector sending int32 was re-declared uint32, and the VDS master was typed unsigned over signed data files. Take pixel_signed from the stream too - it and the width are both carried by the one image_dtype key. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
4e6600a96c |
Build device code for Volta, and document the driver floor
Build Packages / build:windows:nocuda (push) Successful in 14m38s
Build Packages / build:viewer-tgz:cpu (push) Successful in 18m34s
Build Packages / build:viewer-tgz:cuda (push) Successful in 21m44s
Build Packages / build:rpm (ubuntu2404_nocuda) (push) Successful in 23m0s
Build Packages / build:rpm (rocky9_nocuda) (push) Successful in 23m41s
Build Packages / build:rpm (rocky8_nocuda) (push) Successful in 28m36s
Build Packages / build:rpm (ubuntu2204_nocuda) (push) Successful in 28m38s
Build Packages / build:rpm (rocky8_sls9) (push) Successful in 28m54s
Build Packages / build:windows:cuda (push) Successful in 15m45s
Build Packages / build:rpm (rocky9_sls9) (push) Successful in 19m56s
Build Packages / XDS test (durin plugin) (push) Successful in 10m58s
Build Packages / build:rpm (rocky9) (push) Successful in 21m16s
Build Packages / Generate python client (push) Successful in 35s
Build Packages / Build documentation (push) Successful in 1m9s
Build Packages / Create release (push) Skipped
Build Packages / build:rpm (rocky8) (push) Successful in 28m17s
Build Packages / XDS test (JFJoch plugin) (push) Successful in 11m39s
Build Packages / build:rpm (ubuntu2404) (push) Successful in 21m36s
Build Packages / build:rpm (ubuntu2204) (push) Successful in 26m33s
Build Packages / DIALS test (push) Successful in 21m38s
Build Packages / XDS test (neggia plugin) (push) Successful in 10m42s
Build Packages / Unit tests (push) Successful in 2h33m11s
CMAKE_CUDA_ARCHITECTURES had no sm_70 entry, and PTX only ever JIT-compiles forwards, so a V100 had no runnable code in the fatbin at all - every kernel launch failed with "no kernel image is available for execution on the device". Append 70 only for a CUDA 12 toolkit: CUDA 13 removed offline compilation for Volta, so an unconditional entry would break the RHEL 9, Ubuntu and Windows builds. 12.8/12.9 still emit it but warn on every .cu, hence -Wno-deprecated-gpu-targets. The append goes after ENABLE_LANGUAGE(CUDA), where the nvcc version is known, matching the existing sm_121 handling. Verified: all 15 CUDA sources compile for sm_70 (including ffbidx, which already guards on __CUDA_ARCH__ >= 700/800), and cuobjdump shows an sm_70 cubin in the built rugnux binary. Consequence worth documenting: a V100 can only run the artefacts built with CUDA 12 - the RHEL 8 packages and the portable Linux .tgz. Document that alongside the minimum NVIDIA driver of every released artefact (525.60.13 for CUDA 12, 580.65.06 for CUDA 13), which applies because the CUDA runtime is linked statically and cuFFT is bundled, so the driver is the only NVIDIA component the target host must supply. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |