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>
This commit is contained in:
Claude Opus 5
@@ -15,8 +15,11 @@ usage message is the source of truth, not the docs.
|
||||
## Build
|
||||
|
||||
Out-of-source CMake build, C++20, heavy use of `FetchContent` (spdlog, zstd, HDF5,
|
||||
slsDetectorPackage, Catch2, cpp-httplib, libzmq, Ceres, fast-feedback-indexer are downloaded
|
||||
and statically linked — the **first configure needs network access** and is slow).
|
||||
slsDetectorPackage, Catch2, cpp-httplib, libzmq, libtiff, FFTW, Ceres, fast-feedback-indexer are
|
||||
downloaded and statically linked — the **first configure needs network access** and is slow).
|
||||
Two dependencies are deliberately **not** vendored and must come from the system: **ZLIB** and
|
||||
**Eigen3 ≥ 3.4**, both resolved with `find_package` (see the Eigen note in `CMakeLists.txt` for why).
|
||||
libjpeg-turbo is built via `ExternalProject` in `preview/`; libcurl is fetched only for viewer builds.
|
||||
|
||||
```
|
||||
mkdir build && cd build
|
||||
@@ -25,16 +28,24 @@ make -j$(nproc) jfjoch_broker # the main service; build other targets by na
|
||||
```
|
||||
|
||||
Key CMake options:
|
||||
- `JFJOCH_USE_CUDA` (default ON) — GPU path. Needs CUDA ≥ 12.8. Provides the `ffbidx` and `fft`
|
||||
GPU indexers; without it only the CPU `fftw` indexer is available (requires FFTW at configure
|
||||
time, auto-detected). CUDA absence is not a build error.
|
||||
- `JFJOCH_USE_CUDA` (default ON) — GPU path. Needs CUDA ≥ 12.8 (older is warned about and
|
||||
ignored). Provides the `ffbidx` and `fft` GPU indexers; without it only the CPU `fftw` indexer is
|
||||
available. FFTW is fetched and `JFJOCH_USE_FFTW` defined unconditionally, so `fftw` is always
|
||||
there. CUDA absence is not a build error — nvcc is looked for on `PATH`, `CUDA_PATH` and
|
||||
`/usr/local/cuda`.
|
||||
- `JFJOCH_WRITER_ONLY` (default OFF) — builds only the HDF5 writer; skips broker, FPGA,
|
||||
receiver, analysis, tests, frontend.
|
||||
- `JFJOCH_VIEWER_BUILD` (default OFF) — builds the Qt6 `jfjoch_viewer` desktop app.
|
||||
- `JFJOCH_VIEWER_BUILD` (default OFF) — builds the Qt6 `jfjoch_viewer` desktop app in addition to
|
||||
the server stack.
|
||||
- `JFJOCH_VIEWER_ONLY` (default OFF on Linux, **forced ON on Windows/macOS**) — builds only
|
||||
`jfjoch_viewer`, `rugnux` and the libraries they link; skips receiver, FPGA, detector control,
|
||||
tests and the frontend.
|
||||
- `SLS9` (default OFF) — build against slsDetectorPackage 9.2.0 instead of 8.0.2.
|
||||
- `JFJOCH_INSTALL_DRIVER_SOURCE` (default OFF) — install the PCIe driver source for DKMS/RPM.
|
||||
|
||||
The frontend is a separate custom target: `make frontend` (runs `npm ci && npm run build` in
|
||||
`frontend/`). It is not built by default unless installing.
|
||||
The frontend is a separate custom target: `make frontend` (in `frontend/`: `npm ci`,
|
||||
`npm run build`, plus the third-party-licenses, Redoc and Sphinx-docs bundling steps). It is never
|
||||
built automatically — `make install` only copies whatever already sits in `frontend/dist/`.
|
||||
|
||||
## Test
|
||||
|
||||
@@ -49,11 +60,14 @@ cd tests
|
||||
./jfjoch_test -r junit -o report.xml
|
||||
```
|
||||
|
||||
`make jfjoch_hdf5_test` builds the HDF5 write-speed benchmark, also used by CI to produce files
|
||||
that are validated against XDS (Durin/Neggia) and CrystFEL.
|
||||
`make jfjoch_hdf5_test` builds the HDF5 write-speed benchmark (it lives in `tools/`, so the binary
|
||||
is `build/tools/jfjoch_hdf5_test`); CI also uses it to produce files that are validated against XDS
|
||||
(Durin/Neggia), DIALS and CrystFEL. `jfjoch_hdf5_enospc_test` + the `enospc_shim` module test
|
||||
out-of-space handling.
|
||||
|
||||
Lint config is `.clang-tidy` (broad `*` check set with many exceptions; namespaces lower_case,
|
||||
classes CamelCase, global constants UPPER_CASE).
|
||||
There is no lint config in the repo (no `.clang-tidy`/`.clang-format`, no CI lint step) — follow the
|
||||
conventions the code already uses: namespaces lower_case, classes CamelCase, global constants
|
||||
UPPER_CASE.
|
||||
|
||||
## Code style
|
||||
|
||||
@@ -62,10 +76,10 @@ implementation that a reader can verify at a glance. Extra abstraction, speculat
|
||||
clever-but-dense constructs are treated as actively harmful, not as polish. When torn between a
|
||||
tidy abstraction and a flat, obvious version, pick the obvious one.
|
||||
|
||||
This matters most in `image_analysis/pixel_refinement/` (e.g. `PixelRefine.cpp`), which is an
|
||||
experimental prototype where readability is how the physics gets verified — keep it especially
|
||||
plain. **Do not add defensive/unrequested code** (extra validation, rejection heuristics, "just in
|
||||
case" branches) without asking first; if a guard isn't clearly needed, leave it out.
|
||||
This matters most in the experimental analysis code under `image_analysis/`, where readability is
|
||||
how the physics gets verified — keep those parts especially plain. **Do not add
|
||||
defensive/unrequested code** (extra validation, rejection heuristics, "just in case" branches)
|
||||
without asking first; if a guard isn't clearly needed, leave it out.
|
||||
|
||||
Match the surrounding code's idiom, naming, and comment density rather than importing a different
|
||||
style.
|
||||
@@ -115,8 +129,9 @@ cd tests/test_data && python jfjoch_broker_test.py # feeds a test image, sta
|
||||
|
||||
**Data flow (online):** detector → FPGA acquisition (`fpga/`, `acquisition_device/`) →
|
||||
`receiver/` builds full images from per-module FPGA output → `image_pusher/` streams CBOR-encoded
|
||||
images over ZeroMQ → `jfjoch_writer` (`writer/`) consumes the stream and writes NXmx HDF5. The
|
||||
broker also emits a low-rate preview stream and a metadata stream (`preview/`).
|
||||
images over ZeroMQ (or TCP) → the consuming side (`image_puller/`) feeds `jfjoch_writer`
|
||||
(`writer/`), which writes NXmx HDF5. The broker also emits a low-rate preview stream and a
|
||||
metadata stream (`preview/`).
|
||||
|
||||
**Writer file split:** one acquisition produces one `_master.h5` plus many `_data_NNNNNN.h5`
|
||||
files. Dataset-wide metadata (geometry, detector config, ROI/azimuthal definitions — anything
|
||||
@@ -129,7 +144,9 @@ The HDF5 master/data layout is one of three `FileWriterFormat`s (`common/JFJochM
|
||||
all NXmx: **`NXmxLegacy`** (master + `_data_NNNNNN.h5` joined by external links),
|
||||
**`NXmxVDS`** (master + data joined by HDF5 virtual datasets — the default), and
|
||||
**`NXmxIntegrated`** (a single self-contained file, no separate data files). Per-image plugins
|
||||
must work for all three; with `NXmxIntegrated` "master" and "data" are the same file.
|
||||
must work for all three; with `NXmxIntegrated` "master" and "data" are the same file. (The enum
|
||||
also has non-NXmx `DataOnly` and `NoFile`; values 4/5 are retired CBF/TIFF, kept only in the
|
||||
OpenAPI enum for back compatibility.)
|
||||
|
||||
**Two acquisition workflows:** the FPGA-accelerated path (JUNGFRAU at PSI; FPGA does masking,
|
||||
summation, spot finding, ROI/azimuthal integration, compression) and the DECTRIS SIMPLON path
|
||||
@@ -146,30 +163,38 @@ generated API model and internal types.
|
||||
`JFJochImageAnalysis`):
|
||||
- `jfjoch_broker` — online, real-time (FPGA + GPU).
|
||||
- `jfjoch_viewer` — interactive Qt desktop (`viewer/`), results not persisted.
|
||||
- `rugnux` (`tools/rugnux_cli.cpp`, built on the `Rugnux` library in `rugnux/`) — offline batch over
|
||||
a stored HDF5; writes `_process.h5` and `.mtz`/`.cif`/`.hkl`. Merging is on by default (`--no-merge`
|
||||
to disable); `--azint-only` runs only azimuthal integration and `--scale` re-scales/merges the
|
||||
already-integrated reflections in a `_process.h5`. (rugnux = the data-processing half of the system;
|
||||
see `docs/NAMING.md`.)
|
||||
- `rugnux` (`rugnux/rugnux_cli.cpp`, built on the `Rugnux` library in the same directory) — offline
|
||||
batch over a stored HDF5, invoked as `rugnux {<options>} <input.h5>` (it has **no `--help`**; run
|
||||
it with no arguments to print the usage, which is the authority on its flags). Rotation vs stills
|
||||
is auto-detected from the goniometer axis. Merging is on by default (`--no-merge` to disable);
|
||||
merging writes `.mtz`/`.cif`/`.hkl` and skips the bulky `_process.h5` unless `--write-process-h5`,
|
||||
while `--no-merge` writes only `_process.h5`. `--azint-only` runs only azimuthal integration, and
|
||||
`--scale` re-scales/merges the already-integrated reflections in a `_process.h5`. (rugnux = the
|
||||
data-processing half of the system; see `docs/NAMING.md`.)
|
||||
|
||||
**`image_analysis/` pipeline** (subdirs): `spot_finding`, `indexing` (`ffbidx`/`fft` GPU,
|
||||
`fftw` CPU), `lattice_search`, `geom_refinement`, `pixel_refinement`, `bragg_prediction`,
|
||||
`bragg_integration`, `rotation_indexer`, `azint`, `roi`, `scale_merge`. Least-squares refinement
|
||||
uses **Ceres** (fetched, built with miniglog, no MKL, CXX_THREADS). `ffbidx` needs a known cell
|
||||
(`-C`) and suits sparse serial stills; `fft`/`fftw` index de novo and suit strong rotation data.
|
||||
`fftw` CPU), `lattice_search`, `geom_refinement`, `bragg_prediction`, `bragg_integration`,
|
||||
`image_preprocessing`, `azint`, `roi`, `scale_merge`, plus `rotation_indexer/` and
|
||||
`dark_mask_analysis/` compiled straight into `JFJochImageAnalysis`. (`beam_stop/` is an unbuilt
|
||||
prototype — it is in no `CMakeLists.txt`.) Least-squares refinement uses **Ceres** (fetched in
|
||||
`image_analysis/CMakeLists.txt`, built with miniglog, no MKL, no Ceres-CUDA, CXX_THREADS).
|
||||
The indexer is chosen with `-X` (`FFBIDX|FFT|FFTW|Auto|None`, default `Auto`, which resolves to a
|
||||
GPU indexer when one is present and `fftw` otherwise): `ffbidx` wants a known cell (`-C`) and suits
|
||||
sparse serial stills; `fft`/`fftw` index de novo and suit strong rotation data.
|
||||
|
||||
**FPGA** (`fpga/`): `hls/` is the Vitis HLS source (image-analysis kernels), `hls_simulation/`
|
||||
runs that same HLS on CPU for hardware-free testing, `host_library/` is the host-side driver,
|
||||
`pcie_driver/` is the kernel module. The HLS algorithms are documented in
|
||||
runs that same HLS on CPU for hardware-free testing, `hdl/` is the Verilog RTL, `host_library/` is
|
||||
the host-side driver, `pcie_driver/` is the kernel module. The HLS algorithms are documented in
|
||||
`docs/FPGA_DATA_ANALYSIS.md`.
|
||||
|
||||
**Detector control** (`detector_control/`): wrappers for SLS (JUNGFRAU) and DECTRIS SIMPLON
|
||||
(EIGER). **`jungfrau/`**: JUNGFRAU ADU→energy gain/pedestal calibration.
|
||||
|
||||
**Other libs:** `common/` (geometry, diffraction experiment, image buffer, CUDA wrappers — the
|
||||
shared core, linked nearly everywhere), `compression/` (zstd + bitshuffle + sqrt lossy),
|
||||
`frame_serialize/` (CBOR stream codec), `gemmi_gph/` (vendored GEMMI for MTZ/XDS_ASCII I/O),
|
||||
`xds-plugin/` (XDS HDF5 read plugin).
|
||||
shared core, linked nearly everywhere), `compression/` (vendored bitshuffle + LZ4 + zstd; the
|
||||
algorithms are `BSHUF_LZ4`, `BSHUF_ZSTD`, `BSHUF_ZSTD_RLE`, `BSHUF_ZSTD_RLE_HUFF`),
|
||||
`frame_serialize/` (CBOR stream codec), `reader/` (HDF5 dataset read-back), `gemmi_gph/` (vendored
|
||||
GEMMI for MTZ/XDS_ASCII I/O), `xds-plugin/` (XDS HDF5 read plugin).
|
||||
|
||||
## Portability (jfjoch_viewer)
|
||||
|
||||
@@ -190,15 +215,16 @@ portable; don't constrain it for portability's sake.
|
||||
configuration). This non-CUDA path must keep working, but it is the macOS fallback — *not* the
|
||||
intended Windows configuration.
|
||||
|
||||
A self-contained Windows build still needs a few dependencies that the Linux build picks up from
|
||||
the system and that are **not** auto-provided via `FetchContent` (besides Qt, supplied
|
||||
externally): **ZLIB** (pulled by hdf5/libtiff/gemmi/libzmq — a bundled zlib-ng in `ZLIB_COMPAT`
|
||||
mode is the intended fix, presented as the `ZLIB::ZLIB` target so the scattered
|
||||
`find_package(ZLIB)` calls resolve), **libjpeg-turbo** (used by `preview/`; upstream discourages
|
||||
`add_subdirectory`, so bring it in via `ExternalProject`), and **Eigen** (header-only; needed by
|
||||
Ceres, by the analysis libs directly, and by `ffbidx` under CUDA — fetch it and point Ceres'
|
||||
internal `find_package(Eigen3)` at it). Wire each guarded so the Linux build, which finds these on
|
||||
the system, stays unchanged.
|
||||
`JFJOCH_VIEWER_ONLY` is forced ON on Windows and macOS, so a plain configure there already builds
|
||||
just the portable subset. libtiff, libjpeg-turbo (via `ExternalProject` in `preview/`) and libcurl
|
||||
(viewer builds only) are now brought in by the build itself, so they need nothing from the host.
|
||||
Two dependencies are still supplied externally on every platform, deliberately and by decision, not
|
||||
as an open TODO: **ZLIB** and **Eigen** (header-only; needed by Ceres, by the analysis libs
|
||||
directly, and by `ffbidx` under CUDA). Both are resolved with `find_package` — the system package on
|
||||
Linux (`zlib-devel`, `eigen3-devel`), a build prefix pointed at by `CMAKE_PREFIX_PATH`/`Eigen3_DIR`
|
||||
on Windows. Vendoring Eigen through `FetchContent` with `OVERRIDE_FIND_PACKAGE` is specifically
|
||||
ruled out: it segfaults the CMake bundled with Visual Studio (see the long note in `CMakeLists.txt`).
|
||||
Qt is supplied externally as before.
|
||||
|
||||
## OpenAPI is the single source of truth
|
||||
|
||||
@@ -209,13 +235,16 @@ the system, stays unchanged.
|
||||
- TypeScript frontend client → `frontend/src/client/` (hey-api `openapi-ts`, `npm run openapi`).
|
||||
|
||||
When you change `jfjoch_api.yaml`, regenerate the relevant client(s); for a version bump write the new version into `VERSION` and run
|
||||
`update_version.sh` (which reads it and rewrites `frontend/src/version.ts`, `docs/conf.py`, the
|
||||
python client and the Redoc html).
|
||||
`update_version.sh` (which reads it and rewrites the `version:` in the YAML itself,
|
||||
`frontend/src/version.ts`, `frontend/package.json`, `docs/conf.py`, the python client, the Redoc
|
||||
html — and also the FPGA HDL and PCIe-driver version strings). It downloads
|
||||
`openapi-generator-cli.jar` and runs `npm install`, so it needs network access.
|
||||
|
||||
## Frontend
|
||||
|
||||
React 19 + TypeScript + MUI + Vite (`frontend/`). Data layer is generated from the OpenAPI spec
|
||||
(`@hey-api/openapi-ts` → fetch client + TanStack Query hooks + zod schemas). Scripts:
|
||||
React 19 + TypeScript + MUI 6 + Vite 7 (`frontend/`); charts are Plotly (`react-plotly.js`). Data
|
||||
layer is generated from the OpenAPI spec (`@hey-api/openapi-ts` → fetch client + TanStack Query
|
||||
hooks + zod schemas; config in `frontend/openapi-ts.config.ts`). Scripts:
|
||||
`npm start` (dev server), `npm run build` (tsc + vite), `npm run openapi` (regen client),
|
||||
`npm run redocly4broker` (regen `broker/redoc-static.html`).
|
||||
|
||||
@@ -227,29 +256,38 @@ A per-image scalar (e.g. `ice_ring_score`, `bkg_estimate`, `mosaicity`) flows an
|
||||
|
||||
1. **Compute** where the azint profile is finalized: `image_analysis/MXAnalysisWithoutFPGA.cpp` (CPU),
|
||||
`receiver/JFJochReceiverFPGA.cpp` (FPGA), and the offline azint worker in `rugnux/Rugnux.cpp`.
|
||||
2. **Message** (`common/JFJochMessages.h`): `std::optional<float>` in `DataMessage`, `std::vector<float>`
|
||||
in `EndMessage`.
|
||||
3. **CBOR**: encode in `frame_serialize/CBORStream2Serializer.cpp` (DataMessage block *and* END block),
|
||||
decode in `CBORStream2Deserializer.cpp` (both). Optional fields are back-compatible — no version bump.
|
||||
2. **Message** (`common/JFJochMessages.h`): `std::optional<float> <name>` in `DataMessage`; in
|
||||
`EndMessage` **two** members — `std::vector<float> v_<name>` (the per-image array) and an
|
||||
`std::optional<float> <name>` run-mean scalar. Mind the `v_` prefix.
|
||||
3. **CBOR**: encode in `frame_serialize/CBORStream2Serializer.cpp` — one key in the DataMessage block
|
||||
(`SerializeImageInternal`) *and* two in the END block (`SerializeSequenceEnd`: `<name>` and
|
||||
`v_<name>`); decode the same three in `CBORStream2Deserializer.cpp`. Optional fields are
|
||||
back-compatible — no version bump.
|
||||
4. **HDF5** write: `writer/HDF5DataFilePluginMX.{h,cpp}` — an `AutoIncrVector<float>` with reserve /
|
||||
per-image write / `SaveVector("/entry/MX/<name>")` (per-image arrays live in the data-file plugin);
|
||||
plus the NXmx master write in `writer/HDF5NXmx.cpp` (`SaveVectorIfMissing(..., end.<name>)`). HDF5
|
||||
**read-back** (so a stored file re-opens, e.g. in the viewer) is in `reader/HDF5MetadataSource.cpp`,
|
||||
NOT `JFJochHDF5Reader.cpp`: mirror the three `bkgEstimate` sites — master `ReadOptVector`, data-file
|
||||
`ReadVector` into the dataset, and the per-image message population.
|
||||
plus the NXmx master writes in `writer/HDF5NXmx.cpp` (`SaveVectorIfMissing(..., end.v_<name>)` and
|
||||
a `SaveScalar(... "Mean", end.<name>)`). **HDF5 dataset names are camelCase even though the message
|
||||
fields are snake_case** — `bkg_estimate` is stored as `/entry/MX/bkgEstimate` (+ `bkgEstimateMean`).
|
||||
HDF5 **read-back** (so a stored file re-opens, e.g. in the viewer) is in
|
||||
`reader/HDF5MetadataSource.cpp`, NOT `JFJochHDF5Reader.cpp`: mirror the three `bkgEstimate` sites —
|
||||
master `ReadOptVector`, data-file `ReadVector` into the dataset, and the per-image message population.
|
||||
5. **Scan result**: `common/ScanResult.h` (`ScanResultElem`) + `common/ScanResultGenerator.cpp`
|
||||
(copy in `Add`, resize+fill in `FillEndMessage`).
|
||||
6. **Receiver plot**: `common/Plot.h` (`PlotType`) + `common/JFJochReceiverPlots.{h,cpp}` (`StatusVector`
|
||||
+ Clear / AddElement / GetPlots / GetPlotRaw cases).
|
||||
(copy in `Add`, resize+fill in `FillEndMessage`). The name is **not** preserved here — the
|
||||
`bkg_estimate` member is called just `bkg`, and so is the API property in step 7.
|
||||
6. **Receiver plot**: `common/Plot.h` (`PlotType`) + `common/JFJochReceiverPlots.{h,cpp}` — a
|
||||
`StatusVector` member, cleared in `Setup`, fed in `Add`, plus the `GetPlots` / `GetPlotRaw` cases
|
||||
and (if the run-mean scalar of step 2 is wanted) a `GetBkgEstimate`-style accessor.
|
||||
7. **API**: add to the `plot_type` enum and the `scan_result` images schema in `broker/jfjoch_api.yaml`,
|
||||
regenerate the C++ model (`java -jar openapi-generator-cli.jar generate -i broker/jfjoch_api.yaml -o
|
||||
broker/gen -g cpp-pistache-server`) and the frontend client (`cd frontend && npm run openapi`), then
|
||||
wire `broker/OpenAPIConvert.cpp` (`ConvertPlotType` string→enum and the `Convert(ScanResult)` setter).
|
||||
8. **Reader/viewer**: `reader/JFJochReaderDataset.h` + `reader/JFJochHttpReader.cpp` (`GetPlot_i`) and
|
||||
8. **Reader/viewer**: `reader/JFJochReaderDataset.h` + `viewer/JFJochHttpReader.cpp` (`GetPlot_i`) and
|
||||
`viewer/JFJochViewerDatasetInfo.cpp` (combo item + `ExtractMetric`).
|
||||
9. **Frontend**: `frontend/src/components/DataProcessingPlots.tsx` (`MenuItem`) + `DataProcessingPlot.tsx`
|
||||
(y-axis label).
|
||||
10. **Docs**: `docs/CBOR.md`, `docs/HDF5.md`, `docs/CPU_DATA_ANALYSIS.md`.
|
||||
in the same directory (y-axis label in `AxisTypeY`).
|
||||
10. **Docs**: `docs/CBOR.md` and `docs/HDF5.md` name the fields literally; `docs/CPU_DATA_ANALYSIS.md`
|
||||
describes the quantity in prose.
|
||||
|
||||
Gotcha: an existing `build/` dir needs a `cmake .` reconfigure to pick up a newly-added `broker/gen`
|
||||
source file (the source list is a configure-time glob).
|
||||
Gotcha: an existing `build/` dir needs a `cmake .` reconfigure to pick up a newly-added
|
||||
`broker/gen/model` source file — `broker/CMakeLists.txt` collects it with `AUX_SOURCE_DIRECTORY`,
|
||||
a configure-time directory scan. (`gen/api` is include-path-only and needs no reconfigure.)
|
||||
|
||||
Reference in New Issue
Block a user