From 43627e22dc88fd250ce200389a24245a035dfa5a Mon Sep 17 00:00:00 2001 From: Filip Leonarski Date: Tue, 11 Aug 2026 16:33:13 +0200 Subject: [PATCH] docs: a rule for licences and academic credit, and apply it Several methods adopted recently came from other crystallographic packages - the screw-absence test from POINTLESS, MINPK and the profile-fit reweighting from XDS/Otwinowski, the CC1/2 cutoff and merge outlier rejection from DIALS, the per-frame indexing gate from CrystFEL - and nothing in the repository said where such a debt is recorded. The licence side was already worked out (licences beside the vendored code, verbatim texts in licenses/ collected by COLLECT.sh, a row in THIRD_PARTY_NOTICES.md, all installed under share/doc/jfjoch); the credit side was ad hoc. Write the rule into CLAUDE.md. It states the distinction that matters: vendoring or linking someone's CODE creates a LICENCE obligation, discharged in licenses/ and THIRD_PARTY_NOTICES.md; reimplementing an algorithm from a PAPER creates none of that but creates an obligation of academic CREDIT, discharged in docs/ACKNOWLEDGEMENT.md and in a comment at the algorithm. Neither substitutes for the other, and taking both source and paper incurs both. It also fixes the citation form (authors, title, year, journal, volume, pages, verified DOI), and says in-source credit goes at the algorithm, not the file header, in the one-line style the code already uses. Then bring the repository into compliance for the works concerned: docs/ACKNOWLEDGEMENT.md gains a section acknowledging XDS, DIALS, POINTLESS/CCP4, MOSFLM, CrystFEL, GEMMI, the Kabsch/Otwinowski profile fit, the Diederichs & Karplus statistics and the IUCr nomenclature reports, each with a DOI checked against Crossref; docs/CPU_DATA_ANALYSIS.md's reference list gains the ones it was missing; and four algorithms gain a line naming their source where no adjacent comment carried one. No licence change. licenses/ and THIRD_PARTY_NOTICES.md are untouched. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 59 ++++++++++++ common/BraggIntegrationSettings.h | 11 +-- docs/ACKNOWLEDGEMENT.md | 89 +++++++++++++++++++ docs/CPU_DATA_ANALYSIS.md | 9 +- .../BraggIntegrationEngineCPU.cpp | 4 +- .../BraggIntegrationEngineGPU.cu | 4 +- image_analysis/indexing/AnalyzeIndexing.cpp | 5 +- .../scale_merge/SearchSpaceGroup.cpp | 4 + 8 files changed, 176 insertions(+), 9 deletions(-) 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