1254 Commits

This Branch

This Branch

All Branches

Author	SHA1	Message	Date
leonarski_fandClaude Opus 5	ccbc366e2f	reader: read the frame number back with the reflections ... The per-reflection frame number is written to the process file and always has been, but the reader's designated initialiser simply omitted it, so every reflection came back at frame zero. Nothing complained, because zero is a valid frame. It matters because the 3D combine splits a reflection's partials into rocking events by frame contiguity. With every observation claiming frame zero there are no gaps to split on, so a reflection's entire rotation range collapses into ONE event: measured on a rotation dataset, 216066 fulls against 216705 distinct reflections, where the pipeline finds 367416. Forty-two per cent of the observations disappear, the goniometer-frame absorption surface evaluates every observation at a single angle, and the radiation-damage estimate is computed over a run that appears to last no time at all. The reason this survived is that the damage flatters: fewer, better-agreeing observations per reflection give R_meas sixteen per cent lower, ISa thirty per cent higher and a slightly better CC1/2 than the real merge. Anyone re-scaling a stored file was reading numbers that looked better than the pipeline's while standing on less than two thirds of the data, and one radiation-damage figure that was pure artefact. Read it as mandatory rather than optional-with-default, like h/k/l and the intensities: a silent zero is precisely the failure being fixed, and every file this function can read carries the dataset. After the fix the combine reproduces the pipeline exactly. The normal path does not go through this reader and is byte-identical before and after. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-02 13:37:13 +02:00
leonarski_fandClaude Opus 5	4bdb229fb8	spot_finding: find connected components on the GPU ... The spot finder flagged strong pixels on the device and then labelled them on the host, so every frame sent the packed bitmask back - 2.26 MB on a large detector - and the host walked all of it to recover a few hundred pixels. Do the labelling on the device instead: compact the bitmask into a flat-index-sorted list, find each pixel's backward neighbours by binary search, union them lock-free with path halving, then label, accumulate and filter in one kernel. Only the spot list comes back, and only one stream synchronisation per frame. The gain in the ordinary case is modest - about a quarter off per-image spot finding - because the host algorithm is genuinely fast on a normal frame. What justifies it is the frame that is not ordinary. The host labels a sorted sparse list through a window spanning two detector lines, so its cost is quadratic in how many strong pixels share a line. A lit band of detector rows - a hot module, a panel edge - costs 33 ms at two rows and 377 ms at fifteen, all of it under the pixel cap that was supposed to bound this, and none of it maskable when the cause is a diffraction ring rather than a defect: a ring runs tangent to a row at its top and bottom, which is exactly the shape that hurts. The device version is flat at 0.05 to 0.64 ms across every geometry tried, so an online run no longer stalls a quarter of a second on an ice ring. Rejecting an over-cap frame is now free too, since the count is known before any pixel is written. Also label once and filter three times. The per-image minimum-pixel search runs the extraction at three settings, but that setting only decides which components are kept - it does not change the components - so the search itself need not be repeated. This helps the host path as much as the device one. The resolution mask moves to the device as a bit mask, uploaded when the limits change rather than per frame, since the compaction needs it there. Parity is asserted permanently rather than argued: five cases covering realistic frames, occupancy from a hundred pixels to past the cap, the pathological geometries including rings, the resolution mask, and a hundred-repeat determinism check - requiring the same partition, the same spot order, and identical counts. The centroid is a float sum and therefore order-dependent, so the device walks each component from its root in ascending order and fuses its multiply-add the way the host's does; note that whether the host fuses at all depends on the architecture flags, so exact centroid equality is asserted where the compiler fuses and a two-ulp bound otherwise. Making those accumulators integer would remove that dependence entirely and is worth doing separately. Regression set: all 37 crystals identical to the last printed digit. Unit suite passes with the new cases. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-02 13:34:43 +02:00
leonarski_fandClaude Opus 5	e4d70f0e55	image_preprocessing: inline the buffer accessors ... operator[], size(), data() and getBuffer() are one-line accessors that were defined in the .cpp. The build sets no link-time optimisation, so out of line each of them is a real call - once per pixel, from the CPU preprocessor, the CPU azimuthal integrator and the CPU spot finder - and they stop those loops vectorising at all. They show up in a profile directly: about six per cent of a whole azimuthal-integration-only run is spent in the call overhead of two accessors that do nothing but index a vector. Moving them into the header retires 30% fewer instructions on that run and takes the per-image CPU cost on a GPU-less pass from 34.6 to 24.2 ms, with the output bit for bit unchanged - same observation count, same cell, same merge statistics. It is worth nothing on the GPU path, where the image stays on the device, and everything on the paths that have no GPU to fall back on. This also explains a measurement that had been blamed on the pixel mask being a vector<bool>: a microbenchmark of that loop indexed a raw pointer and came out far faster than the same loop in the binary, and the difference was this call, not the mask. Measured properly the mask costs about 14% single-threaded rather than the 41% claimed, and at the thread counts this actually runs at the bit mask is FASTER than the byte mask it was proposed to become, because it moves eight times less traffic and the loop is bandwidth bound. That change should not be made. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-02 11:20:31 +02:00
leonarski_fandClaude Opus 5	639fbb3fbc	indexing: select predicted reflections by partiality, build indexers where it pays ... When more reflections are predicted for a frame than the output can hold, the surplus was dropped by keeping those closest to the Ewald sphere. On the rotation path that quantity is identically zero by construction - the rocking coordinate is chosen so the scattering vector lands exactly on the sphere - so the comparison fell through to h, k and l and the survivors were whichever came first in lexicographic order. Measured on a large cell: every value within one float ulp of zero, and the kept set had a MEAN PARTIALITY BELOW that of the full set, i.e. worse than choosing at random. Rank by partiality instead, which the predictor already computes and which is what the header always claimed was being kept. On the one regression crystal large enough to cross the cap this lifts completeness from 84.8% to 90.2% on the same observations; multiplicity and R_meas move the way they must when the same measurements cover more of reciprocal space. The online path asked for a cap of ten thousand but the truncation was hardcoded to the offline limit, so the broker predicted and integrated up to six times what it could transport and discarded the rest after paying for it. Honour the caller's limit, which also makes the post-integration re-truncation dead code. Indexer pool construction becomes a policy. The online service needs every indexer resident before data arrives, because a cuFFT plan built on the first frame is planning time inside the measurement; spending memory to be ready is the intended trade there and stays the default. Offline there is no such deadline, and a stills run with a known cell was holding a fully allocated FFT indexer per worker that the algorithm resolution can never dispatch - 2.8 GB where 0.4 GB is needed. rugnux and the viewer opt into building on first use; the broker, the receiver and the tests are untouched. This also removes a dangling reference that was latent: the worker held the settings by reference although the pool is routinely constructed from a temporary, which only survived because eager construction finished inside the constructor call. Finally, refuse a first-pass lattice that indexes fewer than a sixth of the validation frames. It fires on nothing in the regression set - the weakest real crystal sits at 22 of 60, more than twice the floor - so it is a backstop, but the failure it prevents is one the set does contain: a dataset with no crystal at all adopts a lattice from its powder rings, integrates every image against it, and dies much later inside the merge complaining about resolution. It now stops in the first pass and says what to try. Regression set: 36 of 37 crystals byte-identical, the exception being the completeness gain above; 34 of 37 space groups, no failures. Full unit suite passes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-02 09:12:27 +02:00
leonarski_fandClaude Opus 5	b47bce7c3b	ci: give the MSVC viewer /arch:AVX, and write down why -march lives in CI ... The Linux jobs already pass -march=x86-64-v3; the MSVC viewer job passed nothing, so it built at the x64 baseline. MSVC has no spelling for the x86-64-v2 level, but /arch:AVX is the nearest and implies SSE4.1/4.2 - which is the part that matters, because below SSE4.1 Eigen has no vectorised round and falls back to one libm call per element. AVX is Sandy Bridge and up, a safe floor for a desktop viewer. The architecture flags stay OUT of CMakeLists on purpose, so a site can build x86-64-v4 on an AVX-512 cluster, or -march=native, or the plain baseline. That is easy to mistake for an oversight and "fix", so say it in CLAUDE.md - together with the consequence that catches anyone profiling: a default local Release build is not what CI or production runs, and the gap is not uniform. GPU-bound work is unaffected, but the CPU and Eigen bound phases - first-pass indexing and scaling/merging - measure about 26% slower without the flags. That is enough to make rounding look like a tenth of all cycles when a real build has it nearly free, and to send a reader at the wrong code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-01 21:58:36 +02:00
leonarski_fandClaude Opus 5	6e805f53c0	image_analysis: stop paying for work that is thrown away ... Three independent costs, each measured, none changing a result. Across the 37-crystal regression set the run time halves (median per crystal 2.0x, total 2.3x) and every crystal's merge statistics are unchanged. The image copy back from the device moved the whole preprocessed frame - 72 MB on a large detector, every frame, per worker - to serve a single host consumer that reads only the strong pixels, at most a few hundred kilobytes of it. Give the buffer a Gather() so that consumer asks for the values it actually wants (a host loop on the CPU, a small kernel on the GPU), and copy the frame back only when a CPU spot finder will genuinely read it. The copy the other way was worse: it came from an unregistered vector, so the driver staged it through its own pinned pool with a host-side memcpy on the calling thread, which does not overlap and collapses under concurrency - 11.6 GB/s at one worker, 1.6 GB/s at eight. That, not any hardware limit, is why throughput stopped improving past four to eight workers. Pinning the decompression buffer once per worker fixes it: on a 18 Mpx dataset the image loop goes from 13.6 to 7.9 ms per image at 32 workers, and 32 workers now beat 8 instead of losing to them. Ceres was computing seventeen partial derivatives where five are free. The per-image rotation refinement frees the beam and the orientation and holds distance, detector angles, rotation axis and cell constant, but the cost function declared all seven blocks, so every residual evaluated in Jet<17> arithmetic. A residual exposing only the two free blocks - the same arithmetic, the constants baked in - halves refinement, and it is exact rather than merely close: dual coordinates evolve independently, so the residuals and the free Jacobian columns are unchanged bit for bit. The merge sorted an index array with a comparator that dereferenced a 1.6 GB array of 72-byte records, i.e. a random walk over memory, single-threaded, twice per two-pass run. Sorting a packed key instead is 2.4x. French-Wilson allocated its integration scratch per reflection and ran serially; it now takes caller-owned scratch and runs over chunks, 4.2x. The correction surfaces re-tested every observation for usability and parity on each of ~22 passes and re-allocated their accumulators each time; bucket the indices once and hoist the buffers. Also convert std::round to std::rint where the rounded value only ever enters a squared residual. The tie rules differ - away from zero against to even - so this is safe exactly where a tie flips the sign but not the magnitude, and unsafe wherever the value becomes a Miller index; those sites keep std::round. Verified over all 2^32 float bit patterns: 8388608 exact ties exist, and the squared residual is bitwise equal for every one of them. Worth little on its own here, because the rounding that dominates is in candidate refinement, where the value is an index and the substitution is not available. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-01 21:35:28 +02:00
leonarski_fandClaude Opus 5	bfb8cb813c	rugnux: report per-image cost honestly instead of per-worker blocked time ... Each stage timer measures wall time inside one worker, so it counts whatever that worker spent blocked on a contended resource - above all the single GPU - as well as its own work. Those waits overlap across workers, so the mean was printed as if it were the per-image cost when it is roughly the per-image cost times the worker count. At the default thread count on a large detector the reported total came out more than twenty times the truth, and single stages were printed as several times the entire per-image budget of the run. That is the one output anyone tuning performance reads, and it sent this investigation at the wrong stage for a while. Divide by the worker count. It is a lower bound - a worker idle rather than blocked is not counted - so rather than hide the remainder, report the image loop's own wall time next to it, and with it the time spent OUTSIDE the loop. Nothing measured the latter before, yet on a rotation run the first-pass indexing and the scaling and merging can be more of the run than the per-image work is: on a large-detector run here it is 5.1 s against 3.0 s. Both figures are for the last pass, and a two-pass rotation run does all of it twice. Also stop printing nan. The per-image indexing and scaling timers are never fed on the two-pass rotation path, because the lattice is forced rather than searched per image and the merge happens outside the loop, so every default rotation run reported "indexing nan scaling nan". A stage that did not run is now simply absent. Measured against the loop's own wall clock on a 18 Mpx dataset: 5% at one worker, 11% at eight, 29% at thirty-two, versus 23x too high before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-01 14:49:45 +02:00
leonarski_fandClaude Opus 5	0ae1a307bc	indexing: complete a rank-deficient direction set, and keep the higher-symmetry setting ... The FFT shortlist could be rank-deficient, and then no cell could be formed at all. FilterFFTResults takes the strongest max_vectors RAW directions and only then prunes ones within 5 degrees of each other, but a single lattice row is sampled by many neighbouring directions of the 16k half-sphere, so thirty raw peaks routinely prune down to four or five distinct directions - the strongest, hence shortest, rows. When a crystal's densest rows share a plane, every surviving direction is coplanar, every triple the reduction forms is degenerate, and the indexer returns nothing. On such a crystal the weak third axis was the eighth distinct direction, at raw rank 78. Keep walking the same magnitude order for up to four more directions that are 5 degrees clear of everything kept, appended after the length sort so the earlier entries hold their positions and the reduction still forms every triple it formed before - the shortlist only gains candidates at its end. That exposed two ways a change of SETTING was mistaken for a different lattice. A centred conventional cell is an exact integer multiple of its primitive one, so the same lattice described two ways differs by that factor: comparing conventional volumes reads a setting change as a sub-cell or a supercell. Both the candidate selection in the rotation indexer and the pass-2 comparison in the driver did exactly that, and between them they discarded a correctly-classified cubic F cell in favour of the body-centred tetragonal description of the very same lattice. Compare primitive volumes in both, as the scheme comparison already did. Fixing the volumes alone was not enough, because the indexed fraction is also biased across crystal systems: a subgroup setting holds fewer cell parameters fixed than its supergroup, so it can never index fewer spots and will always look better by that measure. Where a candidate has a lower lattice point-group order at the same primitive volume - the signature of the same lattice in less symmetry - require it to index markedly better, not merely better, before it displaces the incumbent. A general metric-symmetry promotion was implemented and rejected on evidence: it raised a correct body-centred orthorhombic cell to triclinic and a monoclinic one to C-centred orthorhombic, and no threshold separates the cases, because a false pseudo-orthorhombic degeneracy measured tighter than a true cubic one on obliquity and on alternative-basis axis excess alike. Metric alone cannot decide this; only the intensities can, which is what the space-group search is for. Measured over the 37-crystal regression set: one crystal goes from failing outright to 91% indexed with 91% completeness and a better R_meas than the reference, one keeps the cubic setting it had before, and every other crystal is byte-identical. Full unit suite passes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-01 12:59:34 +02:00
leonarski_fandClaude Opus 5	2238290d6d	rugnux: make spot settings reach rotation indexing, and stop over-claiming ... Three small honesty and cost fixes on the two-pass rotation path. Spot-finding settings did not reach the step that determines the unit cell. The two-pass first pass reuses the spots stored in the file whenever it has them, and reuse is the default, so both sampling schemes and the validation loop ran on acquisition-time spots while only the per-image pass saw the command line. Every --spot-* option was therefore a no-op for the lattice search on any file written by this software, silently, and the lattice was cross-validated against one spot set and applied to another. Giving spot settings now implies re-finding them for the first pass as well, and plain reuse says so in the log. The summary printed a space group and unit cell even when nothing indexed. With a zero indexing rate the cell is whatever the lattice search happened to return, no reflection was ever measured on it, and no output file is written - so stating it as the run's answer claims a result the data do not support. Say that no lattice was determined instead. The second pass re-indexes de novo so the cell comes out self-consistent with the post-refined geometry, and its result was already checked against the first pass - once by the supercell test and once by the centring test - but only after every image had been integrated with it, so a disagreement cost a whole extra pass on a dataset that ended up on the first pass's lattice regardless. Compare them at the point the lattice is adopted instead, using the same two tests and the same fallback. A triclinic de-novo cell is left alone, being the demotion the merge reindexes. Measured over the 37-crystal regression set: merge statistics are unchanged on every crystal (the two that move are the known rotation-indexing non-determinism - one observation in 2.9 million, and a zero-score lattice landing on no partials instead of a few). Crystals whose data were already cached in the reference run are unchanged in wall time. The one dataset that was burning a discarded pass went from three passes to two, 509 s to 198 s, against 0.81x for the same-detector dataset that was already running two passes - so about 214 s of the saving is the removed pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 23:13:11 +02:00
leonarski_fandClaude Opus 5	0b1fb6c870	image_analysis: share the read-only GPU lookup tables per device ... One analysis engine is built per worker thread, and each uploaded its own copy of tables that are pure functions of the detector geometry: the pixel -> azimuthal bin map and the per-pixel corrections (both in AzIntEngineGPU AND again in AdaptiveSpotFinderGPU, from the same mapping), plus the pixel mask. On an 18 Mpx detector that is ~224 MB per worker; with 32 workers ~7 GB of device memory held 32 identical copies. Upload each table once per GPU instead and hand every engine on that device a shared pointer to it. The cache is keyed by (device, source-vector address) because workers are pinned round-robin across GPUs, so on a multi-GPU node each device keeps its own copy - a kernel may only read memory resident on the device it runs on - and the table is freed on the device that allocated it. Entries are held weakly, so a table goes away with the last engine using it. Measured on an 18 Mpx detector, 32 worker threads, 16 GB card: the stills path went from exhausting the card (OOM in de-novo indexing) to 8.6 GB peak, and a normal rotation run from 14.6 GB to 7.4 GB - it had been running within 1.6 GB of the limit, so any larger detector or second GPU consumer would have tipped it over. Per-worker footprint drops 403 -> 173 MB. Merge statistics are unchanged on a six-crystal regression subset, including two-pass runs where the second pass rebuilds the mapping on refined geometry, and wall time is unchanged (13.5-13.8 s vs 13.8-14.1 s). Also take the launch configuration from the current device rather than device 0 in AzIntEngineGPU and ImagePreprocessorGPU: with round-robin pinning, device 0's SM count and shared-memory size can belong to a different card than the one the kernels use. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 18:41:27 +02:00
leonarski_fandClaude Opus 5	1a1e05ad14	spot_finding: run the same two passes on the CPU as on the GPU ... ImageSpotFinderGPU::Detect launches its kernel twice, feeding the first pass's strong-pixel bitmap back in so the second recomputes each local background with those pixels excluded and keeps them strong. The CPU finder ran a single pass, so the two returned different spot lists for the same frame and a dataset processed without a GPU did not match one processed with it. It matters for any spot wide enough to reach into its own 31x31 background box: the spot inflates the mean and variance it is then tested against, so its outer pixels fail the SNR test. On the test image added here - a 5x5 core at 300 counts with a one-pixel ring at 25 - a single pass returns the 25-pixel core and 7500 counts where two passes return the full 49 pixels and 8100. pxl_val also becomes int64_t, matching the GPU's pixel_result signature. It was int32_t, so pxl_val * pxl_val overflowed above 46341 counts even though the surrounding sums were already 64-bit. The new parity test compares PixelCount and Count, not just the centroid, which does not move for a symmetric spot whether or not the ring was picked up; it was confirmed to fail against the old single-pass CPU. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 15:37:22 +02:00
leonarski_fandClaude Opus 5	bc5e5d3a3d	image_pusher: do not hold connections_mutex across sends and joins ... The header has said since it was written that blocking queue operations must never run under connections_mutex; three code paths did exactly that. KeepaliveThread held it while sending a keepalive to every connection, which blocks until the peer-liveness or backpressure timeout - so one half-dead writer socket could stall SendImage and every /statistics poll for up to a minute, from an idle-time heartbeat. AcceptorThread and StartDataCollection held it across RemoveDeadConnections, which joins a writer thread that may itself be inside such a send. RemoveDeadConnections is split in two: DetachDeadConnections unlinks them from the pool under the mutex, which is quick, and CloseDeadConnections tears them down afterwards with the mutex released - safe because they are no longer reachable by anyone else. The keepalive loop copies the pool out and sends outside the lock, the pattern EndDataCollection already used. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 15:37:22 +02:00
leonarski_fandClaude Opus 5	84489b63b4	broker: deactivate even when the last run left an error behind ... Deactivate() called measurement.get() inside the try that guards the power-off, so an exception stored by a previously failed run was rethrown before services.Off() ever ran: the detector stayed powered while the state reported Error, and the operator had no way to turn it off. The future is still reaped - it has to be - but its failure is logged and dropped. It was already reported when it happened, and leaving the detector on is the worse outcome. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 15:36:46 +02:00
leonarski_fandClaude Opus 5	797a28a572	rugnux: correct the stale adaptive-spots default comment ... The declaration still said "stills on, rotation off". Adaptive detection has since been turned on for both workflows - adaptive_spots.value_or(true) - which the comment at the assignment already explains. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 15:03:33 +02:00
leonarski_fandClaude Opus 5	b5b7cf2cf9	docs: say what the CPU prediction path actually does ... BraggPrediction.h claimed the buffer "GROWS to whatever a frame actually predicts, so a large cell is never truncated here". Only the two GPU Calc overrides call GrowCapacity; both CPU predictors stop at max_reflections. The cap is applied inside the h/k/l walk and before the resolution test, so what survives is the low-|h| block, not the reflections nearest the Ewald sphere - a cell large enough to overflow 20000 gives different merged reflections with and without a GPU. Documented rather than silently claimed otherwise. Also removed a paragraph describing a once-per-predictor overflow warning that no longer exists, and fixed the rugnux_cli.cpp path in HDF5.md. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 15:02:57 +02:00
leonarski_fandClaude Opus 5	66e705c0fe	docs: correct the FPGA analysis limits and the square-root coefficient ... The azimuthal bin limit is FPGA_INTEGRATION_BIN_COUNT = 2048, not 1024. There are 16 ROIs, not 64, and the map is a 16-bit per-pixel mask, so a pixel belongs to any subset of them rather than to exactly one. The lossy transform is round(sqrt(N*N*X)) = round(N*sqrt(X)): the HLS squares the sqrtmult register before multiplying. The doc said sqrt(N*X), which is off by sqrt(N), and the register comment claimed the value was "minus one" and "should be square of the coeff" - both wrong, the host writes N and the FPGA squares it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 15:02:11 +02:00
leonarski_fandClaude Opus 5	164f15c903	geom_refinement: stop committing refinements that did not converge ... Four of the seven ceres::Solve calls in image_analysis obtained a Solver::Summary and never looked at it, so a solve that failed numerically had its parameters written back and was reported as success. StillsPartialityRefine and both PostRefine solves already gated on IsSolutionUsable(); this brings the rest to the same contract. IsSolutionUsable() is the right test rather than checking for CONVERGENCE: it accepts a solve that ran out of iterations or wall-clock time but still descended, which is exactly what the real-time callers depend on when they set max_solver_time instead of max_num_iterations. Only FAILURE and USER_FAILURE are rejected. XtalOptimizer checks before the write-back, so a failed refinement now leaves the caller's geom and latt untouched instead of half-updated. GeometryRefiner folds it into result.ok, which previously reported success from spot and frame counts alone. RingOptimizer returns a geometry by value that both callers assign straight back over their input, so it hands back the unchanged reference rather than a diverged beam centre. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 15:02:11 +02:00
leonarski_fandClaude Opus 5	b1740935d8	CLAUDE.md: bring it back in line with the tree ... Verified section by section. The corrections that matter: - image_analysis/pixel_refinement/ is gone (38ea0ec23). The style rule it anchored - no defensive or unrequested code - stays, now attached to the experimental analysis code generally. - rugnux_cli.cpp lives in rugnux/, not tools/ (f737424bd). - There is no .clang-tidy in the tree and never has been; the naming conventions are kept, described as what the code already does. - compression/ has no sqrt codec - the algorithms are BSHUF_LZ4 and the three BSHUF_ZSTD variants. The square-root transform is an FPGA pipeline stage. - jfjoch_hdf5_test is defined in tools/, not tests/. - JFJOCH_VIEWER_ONLY was undocumented, and is forced ON on Windows/macOS. - The per-image-scalar recipe pointed at reader/JFJochHttpReader.cpp, which does not exist (it is viewer/), and missed that EndMessage carries both a per-image vector and a run-mean scalar, so the CBOR END block needs two keys. Added the camelCase-dataset vs snake_case-field trap. - The portability notes described work already done (libjpeg-turbo) and recommended fetching Eigen, which CMakeLists explicitly rules out. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 14:59:13 +02:00
leonarski_fandClaude Opus 5	4ea5e6d01b	viewer: stop rebuilding closed windows on every dataset tick ... The queue-level fix for the live-follow OOM bounded how many datasets are in flight, but not what each tick costs. Three handlers did full-dataset or full-detector work per tick regardless of whether their window was open: - the calibration window copied the whole pixel mask (GetMask returns a reference; it was taken by value), memcpy'd it and ran a full-resolution recolour on the GUI thread; - the image-list window rebuilt one row of eight QStandardItems per image, and then repainted every cell of the model on every frame to move a one-row highlight; - the dataset-info plot was rebuilt twice per tick, because setCurrentIndex fires currentIndexChanged -> comboBoxSelected -> UpdatePlot and the caller then called UpdatePlot again. The first two now defer to showEvent while hidden, following the pattern JFJochViewerReciprocalSpaceWindow::rebuildGL already uses; the highlight repaints only the two rows that change; and the combo is blocked around setCurrentIndex so the plot is built once. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 14:58:35 +02:00
leonarski_fandClaude Opus 5	a6c9f92c24	viewer: cull and cap the saturated-pixel overlay ... DrawSaturation walked the whole saturated set adding two QGraphicsLineItems each, with none of the viewport culling DrawSpots and DrawPredictions do directly above it, and no upper bound - and it is rebuilt on every pan, zoom and frame change. Unlike spots, that set is not bounded by a setting: an over-exposed frame or a missing beamstop saturates a large fraction of the detector, which meant hundreds of thousands of scene items and a multi-second freeze on each mouse drag. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 14:58:35 +02:00
leonarski_fandClaude Opus 5	fb0272023e	scale_merge: apply outlier rejection to the anomalous split on the GPU path ... The GPU merge kernel rejects outliers on the device and keeps a per-full flag there, but only returned the per-group counts. The host array the CPU path fills stayed all zero, and the anomalous I(+)/I(-) accumulator is host-side and unconditional - so with --reject-outliers and a GPU present, the observations the merged IMEAN dropped were still averaged into I(+) and I(-). The same command on a CPU-only host excluded them: the exported anomalous differences depended on whether a GPU was there. R_meas was unaffected, having its own device-side path that reads the flags in place. MergeAccum now hands the per-full flags back so every host-side reduction sees the same rejections. The comment claiming reject_outliers was excluded from the GPU path was never true. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 14:53:17 +02:00
leonarski_fandClaude Opus 5	f8beee7e87	frontend: surface spot-finding upload failures and send sliders on release ... This was the only panel firing the generated call directly, with the rejection routed to console.log. Because the poll then kept returning the unchanged server value - which still equalled lastDownloadedS - the resync effect never fired, so the dashboard showed a threshold the broker was not using, indefinitely and silently. It now goes through useUpload like its siblings, so a failure raises the snackbar, and onError puts the server's value back in the panel. The eight sliders also applied from onChange, which MUI fires for every intermediate position while dragging: one drag across the ice-ring width sent ~200 PUTs plus ~200 forced /statistics refetches, each reconfiguring spot finding on the running acquisition. Dragging now only moves the panel; the value is sent from onChangeCommitted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 14:50:39 +02:00
leonarski_fandClaude Opus 5	16145cd55c	compression: bound the hperf decompressor by the buffer it was given ... JFJochDecompressHperfPtr took source_size and never looked at it: every per-block length was read out of the stream and passed straight to LZ4_decompress_safe/ZSTD_decompress as the source length, with src_ptr advanced by it. The only check happened after the whole buffer had already been walked. A truncated frame, or a block header claiming 0x7fffffff, read far past the end of a heap buffer - reachable from the ZeroMQ CBOR path and from any HDF5 chunk the XDS plugin is handed. block_size == 0 satisfied the "% BSHUF_BLOCKED_MULT" test and then divided at nelements / block_size, and source_size < 12 underflowed source_size - 12 to about 2^64. Both are now rejected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 14:47:01 +02:00
leonarski_fandClaude Opus 5	60528f6ba4	broker: refuse to deactivate while the detector is busy ... Deactivate() holds m for the whole power-off sequence, which is right - nothing else should touch the detector while it is being turned off - but it had no state check, unlike every other entry point. Called during a measurement, calibration or initialisation it waited on measurement.get() while holding m, and those threads re-acquire m to finish: a deadlock that wedged every endpoint, /cancel included. IsRunning() is exactly the set of states with a live background thread, so deactivating stays possible from Error - otherwise a failed initialise would leave no way to power the detector down. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 14:45:45 +02:00
leonarski_fandClaude Opus 5	1aeb7cfbe3	writer: clamp the processing VDS to the images actually written ... The processing VDS mapping is built from the number of images a run set out to process, while total_images comes from the end message and is the number it actually finished. Those differ whenever a run is cancelled or skips an unreadable frame, and the mismatch was a hard throw - which NXmx::Finalize catches by deleting the temporary master, so rugnux lost the entire _process.h5 and every completed image with it. Ctrl-C after 5000 of 100000 images produced no output file at all. Map what was written and drop the remainder instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 14:44:42 +02:00
leonarski_fandClaude Opus 5	d2d1d78545	spot_finding: keep the GPU wave inside the image ... rowsPerWave is rounded up, so with 32 waves the last waves can start at or past the last row: rmin was never clamped and only the drain loop checked front against height. On any detector below about 1500 rows - including the module-converted 500K and 1M geometries and the kernel's own unit tests - the priming and steady-state loops read whole rows past the end of the image buffer, and those garbage rows entered the sliding background window of the bottom rows. Blocks with no rows to write now return before the first __syncthreads (rmin depends only on blockIdx.y, so the block leaves together and the collective ops stay well formed), and both remaining reads are bounded by height. Rows past the end keep the INT32_MIN sentinel, which the window already treats as "not counted". The raw read in the steady-state loop is left as it is: making it apply the prev_out substitution that the other two read sites use would change which pixels are found, which is a separate question from this fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 14:43:49 +02:00
leonarski_fandClaude Opus 5	4a8a8e69bf	lattice_search: give Niggli character 40 its own reindex matrix ... Character 40 carried a verbatim copy of character 35's matrix (0-10 / -100 / 00-1), whose determinant is 1. A C-centred conventional cell needs determinant 2, so a genuine oC lattice was returned as its primitive monoclinic cell while still being labelled Orthorhombic 'C': the refiner then clamped a ~117 degree beta to 90 and prediction dropped half the reflections of a cell that has no centring. International Tables A 3.1.3.1 gives 0-10 / 012 / -100 for character 40. Character 35 is correct as it stands and is left alone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 14:41:46 +02:00
leonarski_fandClaude Opus 5	5e3d580a0e	lattice_search: fix the sign of the Niggli character 9 reindex matrix ... International Tables A 3.1.3.1 gives 100 / -110 / -1-13 for character 9; the last element was -3. With a negative determinant the transform is left-handed and the "conventional" rhombohedral cell is not hexagonal - beta came out around 110-134 degrees instead of 90 and c was far too long. Any R lattice tall enough to reduce to character 9 was affected, and the downstream Trigonal->Hexagonal promotion then forced 90/90/120 onto that wrong cell. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 14:41:25 +02:00
leonarski_fandClaude Opus 5	89208697cb	common: reset the slot status before putting it back on the free list ... release() published the handle through ReleaseSlot and only then wrote status = InPreparation. ReleaseSlot makes the handle available to GetImageSlot immediately, and GetImageSlot hands back this very object without resetting it, so the receiver could observe the stale Sending status and throw "Trying to send image that is not in preparation", aborting the collection - or take the opposite interleaving and leak the slot. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 14:41:13 +02:00
leonarski_fandClaude Opus 5	433370f648	image_pusher: do not release the image slot twice on a failed send ... SendZeroCopy closes the message when zmq_msg_send fails, and closing a message built with zmq_msg_init_data runs its free function - here zmq_socket_free, which already calls release(). The writer thread then released the same slot a second time, under a comment claiming the callback would not run. The second release put a slot back on the free list while the receiver had already taken it for the next image, so two threads wrote the same buffer and the sending/preparation counters drifted permanently. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 14:40:52 +02:00
leonarski_fandClaude Opus 5	abf156a3aa	CBOR: carry ice_ring_score in the END block ... The per-image ice ring score was encoded for every DataMessage but never for the END message, although docs/CBOR.md has always listed it there and both NXmx::EndResultVectors and HDF5MetadataSource expect it. Any dataset written over the stream therefore had no /entry/MX/iceRingScore, and under NXmxIntegrated - where the whole-run vector is the only copy - the score was lost entirely. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 14:40:17 +02:00
leonarski_fandClaude Opus 5	0ca159449f	Bragg integration: integrate as far as the detector reaches, not to a fixed 1.0 A ... BraggIntegrationSettings::DMinLimit_A had a setter that nothing anywhere called, so it was always its 1.0 A default - in rugnux, the viewer and the broker alike, with no option or API field to change it. It feeds the predictor as high_res_A, which discards any reflection with |q| > 1/d_min, so integration simply stopped at 1.0 A however far the detector reached. Five of the 33 rotation test datasets have detectors reaching past it, down to 0.981 A. On one of them, run with no resolution limit, the shell table ended dead at 1.00 A with that shell still at CC1/2 55.6% and <I/sig> 3.4 - cut mid-shell rather than fading out. This branch had already made the sibling limits detector-driven (spot finding, scaling), so the pipeline was finding spots the detector could see and then refusing to integrate them. Make it a std::optional: unset means as far as the detector reaches, a value limits. The limit is only a bound on how far the lattice walk goes, never a second opinion on what is measurable - both predictors independently drop reflections that miss the detector (BraggPrediction.cpp, BraggPredictionRot.cpp) - which is what makes the detector's own reach the right default. rugnux gains --integration-high-resolution (0 = no limit, as for --spot-high-resolution); the derived per-axis prediction range resolves against the same number, so the two cannot drift. Full battery: 30/33 space groups, unchanged from before, 0 failures and the same three known mismatches; 22 of 32 crystals bit-identical and nothing worse than 5 observations in ~500k. The datasets that gain do so because their detector reached past 1.0 A - the effect is understated here because the harness caps each merge at the XDS resolution anyway. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 14:03:34 +02:00
leonarski_fandClaude Opus 5	406c406988	Bragg prediction: one limit per index, not one cube ... Each Miller index is bounded by its OWN axis - |h| <= a/d_min, |k| <= b/d_min, |l| <= c/d_min - so a single half-width has to be sized for the longest axis and then walks the short ones far past anything the resolution cut can keep. Give the predictor max_h, max_k and max_l instead, in all four implementations (CPU and GPU, stills and rotation), and derive each from its own axis. On a 149/83/226 A cell that is 23.1M candidates per frame instead of 94.2M, 4.1x fewer. Results are bit-identical, as they must be - the candidates removed are only ones the |q| <= 1/d_min cut rejected anyway: over six rotation crystals every merged observation count, high-shell CC1/2 and space group matches the cube exactly, 6/6 space groups correct. It buys almost no time, and the earlier claim that the cube cost 22% of that crystal's wall clock was wrong. Removing 4.1x of the candidates moves it 1m58s -> 1m57s, so the whole prediction sweep is ~1% of the run. The 22% that crystal costs relative to a fixed max_hkl of 100 is genuine extra work at max_l = 227: real reflections inside the resolution sphere along the long axis, predicted and integrated either way. Per-axis limits do not reduce that and cannot. The user-facing setting stays a single number: it exists to bound the work, not to describe the crystal, and applies to all three indices when set. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 13:31:34 +02:00
leonarski_fandClaude Opus 5	309bc8aefb	gitignore: processing output and the data it came from ... The working tree accumulates merged reflection files, models and per-image dumps while testing, and they sit in the repository root next to the source. They are user data: a merged .mtz/.cif carries a sample's measured unit cell and its filename usually carries the sample's name, neither of which may enter this repository. Only build*/ and python-client/ were ignored, so a `git add -A` would have picked all of it up - which is exactly what happened while preparing this branch, caught before the commit was made. Ignore the file types rather than rely on everyone typing the right paths, and un-ignore tests/ so checked-in fixtures still work (git add -f for anything else that genuinely belongs). Also widen the rugnux_vs_xds.py output dir to rugnux_cmp*/, which is where the ad-hoc comparison runs land. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 13:31:14 +02:00
leonarski_fandClaude Opus 5	b0e315e73c	Bragg prediction: derive the lattice walk from the cell, and expose it in the API ... Follow-up to making max_hkl a setting: it is now an optional, and unset means "take it from this crystal". The predictor keeps only |q| <= 1/d_min and h = a.q for the real-space axis a, so |h| <= a/d_min exactly - and likewise |k| <= b/d_min and |l| <= c/d_min. max(a,b,c)/d_min therefore bounds all three at once: nothing that could be predicted lies outside it, and nothing inside it is reached by a shorter axis. It applies to rotation and stills alike, both going through the one place the prediction settings are built. Offline (rugnux, viewer) the default is unset, so every crystal gets its own range; --max-hkl overrides it. Online the broker holds a concrete number, because the cost is the cube of it per image and a live acquisition should not have its frame rate decided by whichever sample is mounted: max_hkl joins bragg_integration_settings in the OpenAPI with a default of 100, so an omitted field arrives as that default (the generated model carries it) rather than as "derive it", and the frontend exposes it next to the integration model. Measured against a fixed 100 on six rotation crystals: three are bit-identical, two were being truncated and recover 419k and 5.8k observations with the high-shell CC1/2 going 15.1 -> 25.8% and 52.1 -> 55.3%, and the space group is unchanged 6/6. It reproduces a fixed 200 exactly, which is the bound being tight rather than merely safe. The sixth is worth recording: a 149/83/226 A cell derives 227, and because a single scalar has to cover the longest axis the cube is ~16x what a per-axis box would be - 22% wall clock, for a net 22 observations out of 364k (the per-frame 65536-reflection cap re-selects at the margin when more candidates are offered) and identical CC1/2, ISa and space group. Per-axis limits would remove that; the predictors already map a thread index to h, k and l separately. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 13:10:58 +02:00
leonarski_fandClaude Opus 5	a06c06931f	Bragg prediction: how far to walk the lattice is a setting, not a literal ... max_hkl was hardcoded to 100 at the one place production builds the prediction settings, so the only way to change it was to edit and rebuild - and it is not a constant of the method, it is a property of the cell. An axis is truncated once a/d_min exceeds it: 100 covers a 150 A axis at 1.5 A, but the same axis at 1.0 A, or a 250 A axis anywhere, loses its outermost reflections with nothing said. Move it into BraggIntegrationSettings next to the other prediction/integration parameters and add rugnux --max-hkl (1..511, default 100 - no behaviour change). Like the integration radii and the background trim it stays out of the OpenAPI, so the broker keeps the default it has today and live analysis cannot be handed a range that would not finish; the offline front end, which knows its cell, can ask for more. RugnuxCommandLine emits it when it is not the default. Measured on five rotation crystals at --max-hkl 200: two are bit-identical at no cost, and three were being truncated - one gains 419k observations (+17%) and takes its high-shell CC1/2 from 15.1% to 25.8% for +14% wall clock, the other two gain 12k and 5.8k observations with CC1/2 76.6->82.4% and 52.1->55.3% for +9% and +1%. ISa is unchanged throughout, and no frame overflowed the prediction buffer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 12:00:45 +02:00
leonarski_fandClaude Opus 5	953302a9eb	Spot plot: the resolution axis comes from the detector, not from 1.5 A ... Dropping the fixed spot-finding limit left the reader still generating the spot-vs-resolution plot over shells that stop at 1.5 A, so a stored file reopened in the viewer showed a plot truncated at exactly the limit that was removed - GenerateSpotPlot drops every spot outside its shells. Pass the detector's own maximum resolution, as SpotAnalyze already does. That value is 0 when the geometry gives no scattering angle at all (no distance or no wavelength), and ResolutionShells throws on a non-positive d_min, once per image. There is no resolution axis to plot against in that case, so skip the plot. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 11:51:22 +02:00
leonarski_fandClaude Opus 5	bf866a0d4c	CUDA: the engines' setup copies belong on the engine's stream ... Making the worker streams non-blocking removed the implicit ordering that the constructors were still relying on. Each engine uploads its static inputs - the pixel mask, the pixel-to-bin map, the corrections, the ROI map - with a blocking NULL-stream cudaMemcpy, and then reads them from kernels on its own stream. A pageable host-to-device cudaMemcpy returns once the source has been staged, with the DMA still in flight, and a non-blocking stream no longer waits for the NULL stream. The failure mode is a silently unapplied mask or a stale mapping, not a crash, so it would not have announced itself. Put them on the stream the engine already owns, and synchronise once at the end of the constructor - that is required for the preprocessor, whose source is a local vector, and leaves the others settled rather than in flight for the cost of one one-time sync. The GPU spot-finder test uploaded its image the same way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 11:51:22 +02:00
leonarski_fandClaude Opus 5	a2adc4e021	Stills scaling: an image whose scale collapsed is dropped, not merged unscaled ... Leaving it at G = 1 looked like the conservative choice and is the more damaging of the two errors. The per-image scale enters as rlp/(partiality*G) and multiplies intensity and sigma alike, so substituting 1 for a scale that was really 1/200 of the run median puts the intensities in 200x too low with sigmas 200x too low too - 1/G^2 times the weight they deserve. The merge cannot defend itself against that, because the number that is wrong is the number the weight is built from. And if the collapsed value was instead a failed fit, G = 1 merges the image mis-scaled by an unknown factor. Per-crystal scales on serial stills genuinely span orders of magnitude, unlike frames of one rotation sweep, so both readings are live. An image whose scale is not believable has no usable scale. Write NaN into its image_scale_corr, which every merge path already skips on, so it drops out of the merged intensities, the error model and the statistics consistently. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 11:51:08 +02:00
leonarski_fandClaude Opus 5	da74197dea	Stills partiality: an unmeasurable CC is not a reason to adopt the refined tilt ... The "keep what the crystal came in with" gate required std::isfinite(cc) before it would reject, so a refined model whose CC could not be measured at all was adopted. ImageReferenceCC returns NaN when fewer than 20 reflections clear the partiality cut - which is exactly what a refinement that collapsed the partialities produces, since the cut is on the partialities it just rewrote. The gate therefore failed open on precisely the crystals it exists to catch, and wrote the NaN into image_scale_cc, on which --min-image-cc then drops the image from the merge, the error model and the statistics. Treat a CC that cannot be measured as worse than one that can, so the crystal is put back exactly as it arrived. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 11:51:08 +02:00
leonarski_fandClaude Opus 5	76395539fd	Viewer: the crystal in the settings panel is the crystal the run uses ... "Analyze dataset" cleared the stored cell and space group unless "Use the stored unit cell / space group" was ticked, and that checkbox defaulted off. But the settings panel writes the user's own cell and space group onto the experiment, so a cell typed into the panel was discarded too - while the checkbox label said "stored", implying it came from the file. It also contradicted the dialog next to it: "Refine geometry (stills)" is offered and default-ticked precisely because a cell is present, and the run then removed that cell. The default dialog state on a stills dataset with a known cell ran the bundle adjustment with nothing to anchor on and dropped indexing off ffbidx, which needs a cell, onto de-novo FFT. Drop the checkbox and take the crystal from the panel, which already has exactly the right semantics: "Unit cell known" ticked writes the cell and group, unticked clears both, and a space group of 0 means none. So ticked = -C/-S, unticked = bare rugnux, and what a run will use is always what is on screen. That also keeps the copied command line honest, since RugnuxCommandLine emits -C/-S from the same experiment. The panel is refilled from the file when one is opened, so a finished job's _process.h5 becoming the active snapshot now shows its group and can be cleared, instead of silently pinning every later run to it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 11:50:54 +02:00
leonarski_fandClaude Opus 5	dd30061005	rugnux: two settings the CLI collected and then discarded ... --polarization was applied with the other geometry overrides, but configure_offline_output runs afterwards and calls ApplyRugnuxExperimentDefaults, which sets the polarization factor unconditionally. Every full-analysis run used 0.99 whatever was asked for, so the Lp correction was wrong at a beamline with different polarization. Apply it after the defaults instead, and stop claiming in RugnuxDefaults.h that nothing here is user-selectable. --scale built a bare ScalingSettings and re-derived the rotation/stills split by hand rather than calling RugnuxDefaultScalingSettings, which is what the split was factored out for. It got scale-fulls, smooth-G, min-captured-fraction and outlier rejection right and dropped CaptureUncertaintyCoeff on the floor: 1.0 in the pipeline, 0.0 here. So re-scaling a rotation _process.h5 gave different sigmas and ISa than the run that wrote it - the exact failure the block's own comment says it exists to prevent. Start from the shared defaults and apply the overrides on top, which also picks up --mosaicity and --search-min-zeta, and let -C bind here too. REJECT_OUTLIERS_DEFAULT_NSIGMA had no reader left afterwards. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 11:50:41 +02:00
leonarski_fandClaude Opus 5	196c72a7fe	Bragg prediction: the rotation GPU launch was one plane short in each direction ... The kernel guards against 2*max_hkl+1 and maps thread i to h = i - max_hkl, but the host launched a grid sized 2*max_hkl. The h = k = l = +max_hkl planes were therefore never launched while -max_hkl was, so the GPU predicted an asymmetric subset of what the CPU loop (inclusive on both ends) does. The same bug was fixed on the stills twin when the whole hkl range moved to the GPU; the rotation predictor kept the old expression. It only bites where the cell actually reaches |h| = 100 inside d_min - a ~150 A axis at 1.5 A - so most data never noticed. Over the 33-crystal rotation battery 29 crystals are bit-identical and 4 gain observations, all of them large-cell or high-resolution: +8519, +4693, +901 and +758 observations, with the high-shell CC1/2 up 15.0->15.1%, 52.0->52.2%, 76.3->76.6% and 51.6->52.1%. Nothing is lost anywhere, and R-meas and ISa move by at most 0.01. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 11:50:10 +02:00
leonarski_fandClaude Opus 5	7786fc1af3	Stills geometry refinement: stop sampling once there are enough strong frames ... The first pass sampled min(n, max(refine_frames * 50, 8000)) images to keep the 200 strongest, so any serial run of 10000 frames or fewer indexed every frame TWICE - and 99.3% of the pass was that sampling, the bundle adjust itself taking 0.24 s. The budget is sized for a low-hit-rate dataset; on data that indexes well almost all of it was wasted. Stop once four times the bundle size has been found, which still leaves the "strongest N" selection a real pool and still spans the run, because the sample is equally spaced. On a lysozyme jet dataset that is 1500 frames examined instead of 4000, the same 200 bundled, and the same refined geometry - beam and distance to the pixel, cell to 0.01 A. Warm cache: the pass drops 33 s -> 9.1 s and the whole run 65 s -> 35 s, with CC1/2 and R-meas unchanged inside replicate noise. Stills only - rotation has its own two-pass and returns from this function early. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 09:30:11 +02:00
leonarski_fandClaude Opus 5	2be8680422	CUDA: let worker streams run concurrently ... Every per-thread stream was created with cudaStreamDefault, and the 20 MB raw image upload went to the legacy NULL stream. A NULL-stream operation implicitly synchronises with every blocking stream in the process, so with one engine per worker thread no two workers' GPU work could ever overlap - the whole GPU pipeline ran serially however many threads were asked for. Create the streams non-blocking and put the upload on the engine's own stream. Measured on 2000 serial stills, interleaved, medians of three: 24.6 -> 19.2 s at -N 32 (-22%), 32.7 -> 21.0 s at -N 16 (-36%), CPU utilisation 436-570% -> 723-859%. Output bit-identical - same observations, uniques, completeness, R-meas, CC1/2, error model and cell. The stream is synchronised at the end of the same function, so the ordering the code relies on is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 23:05:11 +02:00
leonarski_fandClaude Opus 5	111cb2f9a6	Stills: re-check the per-image scale after the partiality refine ... ScaleOnTheFly's collapsed-scale guard ran, and then StillsPartialityRefine re-fitted every crystal's scale with no floor and adopted it unconditionally whenever the image had no prior CC - which is exactly the state the guard leaves behind. So the guard was protecting almost nothing. Measured on a lysozyme jet dataset: of 367 images it left unscaled, only 8 were still unscaled in the output, and 53 reached the merge at or below a fiftieth of the run median, the worst at a 4525th; on a second run of the same sample, 297 images, worst at a 75000th. Those intensities are what the merge saw - up to 94x too high in the written file. Run the guard again on the refined scales. It now reports 367 then 51 on that dataset, and the merge improves: R-meas 117.4 -> 110.8%, CC1/2 96.3 -> 96.5%. Measurements confirm the rest of the guard is right as it stands: 0.02 is ~5x below the lowest scale ever seen on an image that correlates with the merge (no image at CC >= 0.4 falls below a tenth of the median), and leaving the image at G = 1 beats both dropping it and replacing its scale with the median - on a run where 30% of images are affected, dropping costs 1.8 CC1/2 and 29% multiplicity. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 23:05:11 +02:00
leonarski_fandClaude Opus 5	07607d3d47	Keep the online reflection cap where the transport can carry it ... Raising the per-image reflection limit to 65536 for offline reprocessing also raised the image-buffer headroom derived from it, and that headroom divides a FIXED total buffer - so every slot grew from compressed+4 MB to compressed+16.7 MB and the receiver's slot count, i.e. how much of a burst it can absorb, fell by about three. Online never needed the raised limit: measured on three serial stills datasets the worst frame predicts 1380 reflections, 14% of even the old cap. So split them, the same way the geometry refinement's stopping rule is split: online keeps the transport-sized 10000, offline gets the full 65536, and the buffer headroom derives from the online one. Both still come from BraggPrediction so the cap, the prediction and the headroom cannot drift apart. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 22:49:16 +02:00
leonarski_fandClaude Opus 5	3e56d96921	Stills partiality: adopt the refined tilt only when it fits better ... RefineOne re-measured the image's correlation to the reference after writing the refined partialities - because --min-image-cc drops images by it - and then ignored what it measured. A crystal the tilt model suits worse than the fixed partiality it replaces kept the refined model anyway, and the refinement is on by default. Compare against the CC the crystal arrived with and put it back untouched when the refinement does not improve it, which is the same state a crystal with too few reflections to fit ends in. Also four things noted in review and left until now: AdaptiveThresholdTest.cpp was listed twice in the test target, AdaptiveThreshold.h was the one header in image_analysis/spot_finding not in its library's source list, CLAUDE.md said update_version.sh rewrites VERSION when it only reads it, and the CHANGELOG did not mention that image_scale_b is gone from the plot_type enum - which breaks a client that asks for that plot. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 22:03:18 +02:00
leonarski_fandClaude Opus 5	b161da05c1	Geometry refinement: bound offline reprocessing by iterations, not the clock ... The per-image refinement stopped on a wall-clock budget (40 ms, and 20 ms for the rotation-only extra pass). Online that is exactly right - the budget is real and an image that overruns it costs the acquisition. Offline it means the same file refines to a different lattice depending on what else the machine was doing at the time, which is not a property reprocessing should have. Bound it by iteration count instead when the caller is offline. IndexAndRefine takes the workflow as a constructor argument: the receiver asks for the wall-clock bound, rugnux and the viewer get the reproducible one. 50 iterations is Ceres' own default; the per-image problem converges well inside it, so it bounds the pathological case rather than the normal one - measured on five battery crystals, every number is unchanged from the timed version. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 20:42:04 +02:00
leonarski_fandClaude Opus 5	a739b156f3	Two-pass: do not adopt a pass whose lattice the reused space group cannot describe ... The second pass re-indexes de novo and can land in a different setting from the first - most often on the PRIMITIVE sub-cell of a centred lattice. The reindex that exists to undo that declines when the metric does not match, and the code then went on to stamp pass 1's group onto the cell regardless. That is not a small error. A C-centred group on an already-primitive cell means the centring absence rule removes half the reflections that genuinely exist, so the merge holds more unique reflections than its own cell can - measured here as "117% complete" with CC1/2 0.62, against the first pass's 92.6% and 0.98, on a cell of exactly half the C-centred volume. Detect the conflict where it happens and feed it to the pass-2 credibility guard rather than acting on it locally: letting the pass re-search its own group instead produced a P1 answer on a crystal XDS and the first pass both call C2, which is a worse outcome than simply not trusting the pass. The completeness and CC1/2 tests stay as the symptom-side net. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-30 19:52:13 +02:00