diff --git a/CLAUDE.md b/CLAUDE.md index bdbdbde1..0d3181f0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -153,6 +153,65 @@ a hard rule, not a preference. When a bug was found on a specific dataset, commit the *behaviour* ("de-novo indexing adopted a spurious axis-multiple supercell"), never the dataset. +## Licences and academic credit + +Two different obligations, discharged in two different places; neither substitutes for the other. + +- **Vendoring or linking someone's CODE** creates a **licence** obligation — `licenses/` and + `THIRD_PARTY_NOTICES.md`. +- **Reimplementing an algorithm from a PAPER** creates no licence obligation at all, but it creates + an obligation of **academic credit** — `docs/ACKNOWLEDGEMENT.md` and a comment at the algorithm. + +Taking someone's source *and* following their paper incurs both. Do both. + +**Licences.** Vendored code keeps the upstream licence file beside it (`compression/lz4/LICENSE`, +`gemmi_gph/LICENSE.txt`, …) and the adapted file's own header names the upstream URL and licence +(`image_analysis/spot_finding/StrongPixelSet.cpp` is the model). Then: add the path to +`licenses/COLLECT.sh` and run `bash licenses/COLLECT.sh`, which writes the verbatim text to +`licenses/.txt` (committed, so a source checkout ships the notices without a build); and +add a row to the root `THIRD_PARTY_NOTICES.md` — component, path, copyright, SPDX id, link to the +licence text. Both are required; a licence text nobody lists is not attribution. +`docs/THIRD_PARTY_NOTICES.md` is **generated** from the root file by `update_version.sh` — never +hand-edit it. `CMakeLists.txt` installs `LICENSE`, `THIRD_PARTY_NOTICES.md` and all of `licenses/` +into `share/doc/jfjoch` for every package component, so nothing further is needed to reach the built +product. + +A **build-time dependency** (`FetchContent`/`ExternalProject`, statically linked) is handled the +same way with one difference: there is no in-tree copy, so `COLLECT.sh` reads its licence out of the +populated `_deps/` tree — which means that collection step needs a configured build — and the row +goes in the "Fetched at build time" table instead. npm dependencies need nothing by hand: `npm run +licenses` regenerates `frontend/dist/THIRD_PARTY_LICENSES.txt` as part of `make frontend`. + +**Acknowledgements.** The acknowledgement of an external *work* — a paper, a program, an approach +taken from someone else — goes in **`docs/ACKNOWLEDGEMENT.md`**: a short paragraph saying what was +taken and from whom, with the citation. If the method is also described in +`docs/CPU_DATA_ANALYSIS.md`, add the paper to that page's `## References` list as well. + +**Citation form.** Verify every DOI. If you cannot verify one, write the reference without it and +say so — never invent a DOI. + +- Paper — authors, "title" (year), journal, volume, pages, DOI as a link where one exists: + ``` + P. R. Evans, "An introduction to data reduction: space-group determination, scaling and intensity + statistics" (2011), Acta Cryst. D67, 282-292 [doi:10.1107/S090744491003982X](https://doi.org/10.1107/S090744491003982X). + ``` +- Software package — name and link, plus its canonical citation paper if it has one: + ``` + [DIALS](https://dials.github.io/): G. Winter, D. G. Waterman, J. M. Parkhurst et al., "DIALS: + implementation and evaluation of a new integration package" (2018), Acta Cryst. D74, 85-97 + [doi:10.1107/S2059798317017235](https://doi.org/10.1107/S2059798317017235). + ``` + +**In-source credit** belongs **at the algorithm** — the function, the loop, or the setting it +governs — not in the file header, which carries only SPDX. One line: program and/or author, journal, +volume, pages, year. No DOI, no URL. The prevailing style: + +```cpp +// Following Kabsch (2010) Acta Cryst. D66, 133-144 +``` + +Check first and do not duplicate a credit an adjacent comment already carries. + ## Local end-to-end run (no detector / no FPGA) The FPGA HLS logic can be simulated on the CPU (`HLSSimulatedDevice`), so the full software diff --git a/common/BraggIntegrationSettings.h b/common/BraggIntegrationSettings.h index a2580ddd..6b768748 100644 --- a/common/BraggIntegrationSettings.h +++ b/common/BraggIntegrationSettings.h @@ -15,11 +15,12 @@ enum class IntegratorMode { BoxSum, ProfileGaussian, ProfileEmpirical }; // What the integrator does about a signal region shared with a neighbouring reflection. Off is the // historical behaviour: a neighbour's signal is kept out of this reflection's BACKGROUND ring, but // nothing keeps it out of the reflection's own SIGNAL region, so on a dense pattern a crowded -// reflection reads high. Reject is XDS's MINPK - drop the reflection when too little of its expected -// profile is cleanly its own. Exclude - the default - drops only the shared PIXELS from the fit; a -// profile fit is the amplitude of a normalised profile, so leaving pixels out renormalises it by -// construction and the reflection is kept unbiased rather than discarded. A box sum has no profile -// to renormalise with, so Exclude does nothing there; only Reject acts on it. +// reflection reads high. Reject is XDS's MINPK (Kabsch, Acta Cryst D66, 133-144 (2010)) - drop the +// reflection when too little of its expected profile is cleanly its own. Exclude - the default - +// drops only the shared PIXELS from the fit; a profile fit is the amplitude of a normalised profile, +// so leaving pixels out renormalises it by construction and the reflection is kept unbiased rather +// than discarded. A box sum has no profile to renormalise with, so Exclude does nothing there; only +// Reject acts on it. enum class OverlapMode { Off, Reject, Exclude }; // The hkl half-width the broker bootstraps when a config carries no bragg_integration block. Matches diff --git a/docs/ACKNOWLEDGEMENT.md b/docs/ACKNOWLEDGEMENT.md index 043a1f43..1deaa9b4 100644 --- a/docs/ACKNOWLEDGEMENT.md +++ b/docs/ACKNOWLEDGEMENT.md @@ -22,3 +22,92 @@ over a sorted hit list, resolved by a parallel union-find. traccc is MPL-2.0; se [THIRD_PARTY_NOTICES.md](THIRD_PARTY_NOTICES.md). This software uses Viridis, Magma and Inferno colormaps from Matplotlib under its BSD-compatible license + +## Crystallographic methods adopted from other packages + +The analysis pipeline reimplements methods first published, and in most cases first implemented, by +other crystallographic software. The code below is Jungfraujoch's own; the methods are theirs, and +are acknowledged here. Where a package's source was consulted this is said explicitly. None of these +packages is linked or vendored, with the single exception of GEMMI (see +[THIRD_PARTY_NOTICES.md](THIRD_PARTY_NOTICES.md)). + +**[XDS](https://xds.mr.mpg.de/)** — rotation geometry and notation, the reciprocal Lorentz and +partiality treatment, the maximum-likelihood mosaicity estimate, the `MINPK` criterion for rejecting +a reflection whose predicted profile is not cleanly its own, and the intensity-based test for a +centred lattice. W. Kabsch, "XDS" (2010), Acta Cryst. D66, 125-132 +[doi:10.1107/S0907444909047337](https://doi.org/10.1107/S0907444909047337); W. Kabsch, "Integration, +scaling, space-group assignment and post-refinement" (2010), Acta Cryst. D66, 133-144 +[doi:10.1107/S0907444909047374](https://doi.org/10.1107/S0907444909047374). + +**Profile fitting** with reweighted, de-biased variances is the Kabsch/Otwinowski iteration, from the +second XDS paper above and from Z. Otwinowski and W. Minor, "Processing of X-ray diffraction data +collected in oscillation mode" (1997), Methods Enzymol. 276, 307-326 +[doi:10.1016/S0076-6879(97)76066-X](https://doi.org/10.1016/S0076-6879%2897%2976066-X). + +**[DIALS](https://dials.github.io/)** — the resolution cutoff from the CC1/2 fall-off, per-observation +outlier rejection at merge, the scaling error model, and the treatment of a reflection whose +background is contaminated. Its published behaviour, and in places its source, settled several +choices here. G. Winter, D. G. Waterman, J. M. Parkhurst et al., "DIALS: implementation and +evaluation of a new integration package" (2018), Acta Cryst. D74, 85-97 +[doi:10.1107/S2059798317017235](https://doi.org/10.1107/S2059798317017235); D. G. Waterman, +G. Winter, R. J. Gildea et al., "Diffraction-geometry refinement in the DIALS framework" (2016), +Acta Cryst. D72, 558-575 [doi:10.1107/S2059798316002187](https://doi.org/10.1107/S2059798316002187); +J. Beilsten-Edmands, G. Winter, R. Gildea et al., "Scaling diffraction data in the DIALS software +package: algorithms and new approaches for multi-crystal scaling" (2020), Acta Cryst. D76, 385-399 +[doi:10.1107/S2059798320003198](https://doi.org/10.1107/S2059798320003198); J. M. Parkhurst, +G. Winter, D. G. Waterman et al., "Robust background modelling in DIALS" (2016), J. Appl. Cryst. 49, +1912-1921 [doi:10.1107/S1600576716013595](https://doi.org/10.1107/S1600576716013595). + +**[POINTLESS](https://www.ccp4.ac.uk/)** (CCP4) — the space-group search. Stage A scores each +candidate rotation operator by the correlation of I(h) with I(Rh); the screw-axis test scores a +predicted-absent class against the rest of its own axial row rather than against a global mean or a +fixed cut, and lets confidence fall away with the number of axial reflections instead of refusing +below a count. P. Evans, "Scaling and assessment of data quality" (2006), Acta Cryst. D62, 72-82 +[doi:10.1107/S0907444905036693](https://doi.org/10.1107/S0907444905036693); P. R. Evans, "An +introduction to data reduction: space-group determination, scaling and intensity statistics" (2011), +Acta Cryst. D67, 282-292 [doi:10.1107/S090744491003982X](https://doi.org/10.1107/S090744491003982X); +P. R. Evans and G. N. Murshudov, "How good are my data and what is the resolution?" (2013), Acta +Cryst. D69, 1204-1214 [doi:10.1107/S0907444913000061](https://doi.org/10.1107/S0907444913000061); +J. Agirre, M. Atanasova, H. Bagdonas et al., "The CCP4 suite: integrative software for macromolecular +crystallography" (2023), Acta Cryst. D79, 449-461 +[doi:10.1107/S2059798323003595](https://doi.org/10.1107/S2059798323003595). + +**[MOSFLM](https://www.mrc-lmb.cam.ac.uk/mosflm/)** — the Rossmann FFT autoindexing algorithm and +post-refinement practice, including which parameters are safe to refine per image and which must be +refined over a wedge. A. G. W. Leslie and H. R. Powell, "Processing diffraction data with MOSFLM" +(2007), in *Evolving Methods for Macromolecular Crystallography*, NATO Science Series II, vol. 245, +41-51 [doi:10.1007/978-1-4020-6316-9_4](https://doi.org/10.1007/978-1-4020-6316-9_4); +T. G. G. Battye, L. Kontogiannis, O. Johnson, H. R. Powell and A. G. W. Leslie, "iMOSFLM: a new +graphical interface for diffraction-image processing with MOSFLM" (2011), Acta Cryst. D67, 271-281 +[doi:10.1107/S0907444910048675](https://doi.org/10.1107/S0907444910048675); H. R. Powell, +T. G. G. Battye, L. Kontogiannis, O. Johnson and A. G. W. Leslie, "Integrating macromolecular X-ray +diffraction data with the graphical user interface iMosflm" (2017), Nat. Protoc. 12, 1310-1325 +[doi:10.1038/nprot.2017.037](https://doi.org/10.1038/nprot.2017.037). + +**[CrystFEL](https://www.desy.de/~twhite/crystfel/)** — spot finding, the three-ring integration +region, the serial/stills processing model, and the per-frame indexing acceptance test +(`indexing_peak_check()` in `peaks.c`). T. A. White, R. A. Kirian, A. V. Martin, A. Aquila, K. Nass, +A. Barty and H. N. Chapman, "CrystFEL: a software suite for snapshot serial crystallography" (2012), +J. Appl. Cryst. 45, 335-341 [doi:10.1107/S0021889812002312](https://doi.org/10.1107/S0021889812002312). + +**[GEMMI](https://github.com/project-gemmi/gemmi)** — symmetry operations, unit-cell and +structure-factor machinery, and MTZ / XDS_ASCII I/O. Vendored in `gemmi_gph/`, so it also carries a +licence obligation. M. Wojdyr, "GEMMI: A library for structural biology" (2022), J. Open Source +Softw. 7, 4200 [doi:10.21105/joss.04200](https://doi.org/10.21105/joss.04200). + +**Data-quality statistics** follow the established conventions rather than any one program: R_meas +and R_pim, CC1/2 and CC\*, and the reporting of I/sigma(I). K. Diederichs and P. A. Karplus, "Improved +R-factors for diffraction data analysis in macromolecular crystallography" (1997), Nat. Struct. Biol. +4, 269-275 [doi:10.1038/nsb0497-269](https://doi.org/10.1038/nsb0497-269); P. A. Karplus and +K. Diederichs, "Linking crystallographic model and data quality" (2012), Science 336, 1030-1033 +[doi:10.1126/science.1218231](https://doi.org/10.1126/science.1218231); K. Diederichs and +P. A. Karplus, "Better models by discarding data?" (2013), Acta Cryst. D69, 1215-1222 +[doi:10.1107/S0907444913001121](https://doi.org/10.1107/S0907444913001121). + +**Uncertainty conventions** follow the IUCr Commission on Crystallographic Nomenclature: +D. Schwarzenbach, S. C. Abrahams, H. D. Flack et al., "Statistical descriptors in crystallography: +Report of the IUCr Subcommittee on Statistical Descriptors" (1989), Acta Cryst. A45, 63-75 +[doi:10.1107/S0108767388009596](https://doi.org/10.1107/S0108767388009596); D. Schwarzenbach, +S. C. Abrahams, H. D. Flack, E. Prince and A. J. C. Wilson, "Statistical descriptors in +crystallography. II. Report of a Working Group on Expression of Uncertainty in Measurement" (1995), +Acta Cryst. A51, 565-569 [doi:10.1107/S0108767395002340](https://doi.org/10.1107/S0108767395002340). diff --git a/docs/CPU_DATA_ANALYSIS.md b/docs/CPU_DATA_ANALYSIS.md index 5b34550d..2c00b97e 100644 --- a/docs/CPU_DATA_ANALYSIS.md +++ b/docs/CPU_DATA_ANALYSIS.md @@ -31,7 +31,14 @@ The methods are inspired and reuising solutions implemented in: - A. T. Brünger, "Free R value: a novel statistical quantity for assessing the accuracy of crystal structures", *Nature* **355** (1992), 472-475 (R-free cross-validation). - M. Wojdyr, "GEMMI: A library for structural biology", *J. Open Source Softw.* **7** (2022), 4200 (model / structure-factor / map machinery used in §14). - J. P. Wright, "Experiences with GPU decompression for bitshuffle + LZ4 data", HDF5 User Group meeting (2021), and [github.com/jonwright/bslz4decoders](https://github.com/jonwright/bslz4decoders) (device-side decoding of bitshuffle+LZ4 images, §0). -(list is not exhaustive) +- Z. Otwinowski & W. Minor, "Processing of X-ray diffraction data collected in oscillation mode", *Methods Enzymol.* **276** (1997), 307-326 (reweighted, de-biased profile-fit variances). +- G. Winter et al., "DIALS: implementation and evaluation of a new integration package", *Acta Cryst.* **D74** (2018), 85-97, and J. Beilsten-Edmands et al., *Acta Cryst.* **D76** (2020), 385-399 (CC1/2 resolution cutoff, merge outlier rejection, scaling error model). +- P. Evans, "Scaling and assessment of data quality", *Acta Cryst.* **D62** (2006), 72-82, and P. R. Evans, *Acta Cryst.* **D67** (2011), 282-292 (POINTLESS: operator-by-operator point-group scoring, and the axial-zone screw-absence test). +- A. G. W. Leslie & H. R. Powell, "Processing diffraction data with MOSFLM" (2007), NATO Science Series II **245**, 41-51 (post-refinement practice: what is refined per image and what over a wedge). +- K. Diederichs & P. A. Karplus, *Nat. Struct. Biol.* **4** (1997), 269-275, and P. A. Karplus & K. Diederichs, *Science* **336** (2012), 1030-1033 (R_meas / R_pim, CC1/2 and CC\*). +- IUCr Commission on Crystallographic Nomenclature, "Statistical descriptors in crystallography", *Acta Cryst.* **A45** (1989), 63-75, and *Acta Cryst.* **A51** (1995), 565-569 (uncertainty conventions). + +(list is not exhaustive; the full citations, with DOIs, are in [ACKNOWLEDGEMENT.md](ACKNOWLEDGEMENT.md)) ## 0. Getting the image onto the GPU: device-side bitshuffle+LZ4 decoding diff --git a/image_analysis/bragg_integration/BraggIntegrationEngineCPU.cpp b/image_analysis/bragg_integration/BraggIntegrationEngineCPU.cpp index 15201c36..2916dd84 100644 --- a/image_analysis/bragg_integration/BraggIntegrationEngineCPU.cpp +++ b/image_analysis/bragg_integration/BraggIntegrationEngineCPU.cpp @@ -402,7 +402,9 @@ std::vector BraggIntegrationEngineCPU::RunImpl(const Sampler &img, shell_sigma2[s] = widths(shell_mom[s]); } - // --- Pass B: profile-fit each reflection (Kabsch, de-biased variance v = B + I*P; iterate). --- + // --- Pass B: profile-fit each reflection (Kabsch, de-biased variance v = B + I*P; iterate). The + // reweighting is the Kabsch/Otwinowski iteration: Kabsch, Acta Cryst D66, 133-144 (2010); + // Otwinowski & Minor, Methods Enzymol 276, 307-326 (1997). --- std::vector Pbuf; for (size_t i = 0; i < npredicted; ++i) { const auto &rh = rough[i]; diff --git a/image_analysis/bragg_integration/BraggIntegrationEngineGPU.cu b/image_analysis/bragg_integration/BraggIntegrationEngineGPU.cu index 38a35b16..49229085 100644 --- a/image_analysis/bragg_integration/BraggIntegrationEngineGPU.cu +++ b/image_analysis/bragg_integration/BraggIntegrationEngineGPU.cu @@ -441,7 +441,9 @@ __global__ void radial_correct(const float *rad_sum, const int *rad_cnt, int n_r I_o[i] = isum_a[i] - (float) ninner_a[i] * bkg_new; } -// --- Pass B Kabsch profile fit: I = sum P(c-B)/v over sum P^2/v, v = B + max(I,0)P (iterate). +// --- Pass B Kabsch profile fit: I = sum P(c-B)/v over sum P^2/v, v = B + max(I,0)P (iterate). The +// reweighting is the Kabsch/Otwinowski iteration: Kabsch, Acta Cryst D66, 133-144 (2010); +// Otwinowski & Minor, Methods Enzymol 276, 307-326 (1997). // One block per reflection; the (possibly elongated) profile is built in shared memory. --- __global__ void fit(const int32_t *img, const uint32_t *owner, const float *px_x, const float *px_y, const int *cx_a, const int *cy_a, const float *dd, const unsigned long long *invd2mm, diff --git a/image_analysis/indexing/AnalyzeIndexing.cpp b/image_analysis/indexing/AnalyzeIndexing.cpp index 23d82a9e..22b10249 100644 --- a/image_analysis/indexing/AnalyzeIndexing.cpp +++ b/image_analysis/indexing/AnalyzeIndexing.cpp @@ -422,7 +422,10 @@ bool AnalyzeIndexing(DataMessage &message, bool outcome = false; // Minimum fraction of the in-resolution spots a candidate lattice must index to be accepted. // Lowering it admits weaker/sparser crystals (more real ones on flooded XFEL frames, but also more - // spurious lattices that a downstream merge-consistency gate must remove). + // spurious lattices that a downstream merge-consistency gate must remove). The gate is a stills + // notion (CrystFEL, White et al., J. Appl. Cryst. 45, 335-341 (2012)): XDS, MOSFLM and DIALS index + // once over the sweep and then integrate every frame from that lattice - none of them re-decides + // per frame whether a frame may be integrated. constexpr float min_frac = 0.20f; if (nspots_indexed >= viable_cell_min_spots && nspots_indexed >= std::lround(min_frac * nspots_ref)) { auto uc = latt.GetUnitCell(); diff --git a/image_analysis/scale_merge/SearchSpaceGroup.cpp b/image_analysis/scale_merge/SearchSpaceGroup.cpp index 312fba78..f9a61238 100644 --- a/image_analysis/scale_merge/SearchSpaceGroup.cpp +++ b/image_analysis/scale_merge/SearchSpaceGroup.cpp @@ -97,6 +97,10 @@ namespace { // How unlikely the predicted-absent class would be if the screw did not exist, in nats. // + // Scoring the absence against the rest of its own axial row follows the POINTLESS zone test + // (Evans, Acta Cryst D67, 282-292 (2011), App. A3); the Beta tail here is an analytic null in + // place of its control transforms. + // // Under "no screw" the absent class and the rest of its axial row are both Wilson-distributed with // the SAME mean, so with each absent intensity expressed in units of its row's control mean, the // fraction T = sum_u / (sum_u + n_control) follows Beta(n_absent, n_control) exactly. The row's own