From 6c0b3f58fea7c54237b75930d48b7b150a3101d7 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 12:24:28 +0000 Subject: [PATCH] docs: move docs/open-items.md into the issues and docs/parity.md Every open item already has an issue (open, or closed as not planned and listed on mac4DSTEM/macEM-suite#94). The limits and decisions that are not work items move to a "Known limits and decisions" section at the end of docs/parity.md; the README, status and benchmark pages point there and to the issues. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VkKZEUoL9tkBoyymw9Fcus --- README.md | 9 ++++---- ROADMAP.md | 4 ++-- docs/benchmarks.md | 6 ++--- docs/open-items.md | 57 ---------------------------------------------- docs/parity.md | 38 ++++++++++++++++++++++++++++++- docs/status.md | 4 ++-- 6 files changed, 48 insertions(+), 70 deletions(-) delete mode 100644 docs/open-items.md diff --git a/README.md b/README.md index 4d82681..0f2b731 100644 --- a/README.md +++ b/README.md @@ -17,7 +17,7 @@ Swift 6 strict concurrency on `Foundation`, `simd`, `CryptoKit`, Metal (the ACOM module. The one accelerated path, `ACOMMatcher.matchAll(..., backend: .metal)`, keeps the CPU path as its reference and is checked against it. **Status: 0.x** ([semantic versioning](https://semver.org): a minor release may change the API, and every release has a -`CHANGELOG.md` entry with a migration note where needed). Pin an exact tag and bump on purpose. +`CHANGELOG.md` entry with a migration note where needed). Commit your `Package.resolved` and read the CHANGELOG before an update. **Platform.** Apple platforms today (macOS 15 or later, iOS 18 or later). Linux is not supported yet: `DiffractoCore` imports `CryptoKit` and `DiffractoOrientation` depends on ImagoKit, which uses Accelerate. @@ -38,7 +38,7 @@ One SwiftPM product per module; a module imports only the ones above it in this ```swift // Package.swift dependencies: [ - .package(url: "https://github.com/mac4DSTEM/DiffractoKit", exact: "0.11.0"), // x-release-please-version + .package(url: "https://github.com/mac4DSTEM/DiffractoKit", from: "0.11.0"), // x-release-please-version ], targets: [ .target(name: "MyApp", dependencies: [ @@ -261,18 +261,17 @@ and ported from: pymatgen, spglib, py4DSTEM and, for the orientation work, pyxem - One real-data test (`testRinglessFitOnARealMeanPatternFallsBackToTheConic`, a Ni65Cu35 mean pattern) is disabled until that dataset is published with the tests. - Policies not yet parameters (profile-fit plausibility bounds, cell volume 1e-6) remain. -- The open list, worst first, with causes: `docs/open-items.md`; timings against Python: `docs/benchmarks.md`. +- Open work: the GitHub issues; known limits with their causes: `docs/parity.md`; timings against Python: `docs/benchmarks.md`. ## Status and documents -0.x: the API may change between minor versions; pin an exact tag. Numbers are held to the Python reference within its own tolerance; `docs/parity.md` lists each function, its state and its tolerance. +0.x: the API may change between minor versions; commit `Package.resolved` and read the CHANGELOG before an update. Numbers are held to the Python reference within its own tolerance; `docs/parity.md` lists each function, its state and its tolerance. | Document | What | |---|---| | `CHANGELOG.md` | what changed in each release, with migration notes | | `docs/parity.md` | the checklist against pymatgen, py4DSTEM, pyxem / diffsims / orix | | `docs/benchmarks.md` | the per-function timings against the Python counterparts and the backend decision | -| `docs/open-items.md` | what is open, with causes | | `docs/orientation-symmetry.md` | orientation conventions | | `tools/consumer-check/README.md` | the separate package that uses the public API only | | `tools/readme-consumer/README.md` | the separate package that compiles and runs exactly this README's program | diff --git a/ROADMAP.md b/ROADMAP.md index d6fbd06..f4509df 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -1,7 +1,7 @@ # Roadmap -Where DiffractoKit is going next, in order. `docs/parity.md` says what is held today, `docs/open-items.md` what is known to be -open, and the GitHub issues track each piece of work. The numbers below are the ones `docs/parity.md` cites as "ROADMAP item n". +Where DiffractoKit is going next, in order. `docs/parity.md` says what is held today and its known limits, and the GitHub +issues track each piece of work. The numbers below are the ones `docs/parity.md` cites as "ROADMAP item n". 1. **Diagnose before fixing.** Phase matching (`minimumVectors` = 2 makes "matrix" by exclusion; the cross-phase winner ignores completeness), the ACOM mirror (a mirrored win carries `inPlaneAngle` + π with no flag; some off-grid diff --git a/docs/benchmarks.md b/docs/benchmarks.md index 1619666..0a62e20 100644 --- a/docs/benchmarks.md +++ b/docs/benchmarks.md @@ -12,7 +12,7 @@ Float64 unless the function is Float32 by design (ACOM, ellipse pattern pixels). | Python | 3.12.14, NumPy 2.5.4, SciPy 1.18.1, in two virtual environments: py4DSTEM 0.14.19 in one; pymatgen-core 2026.10.16 (the code; its `pymatgen` metapackage dist-info reads 2026.9.24), orix 0.15.0 and spglib 2.8.0 in the other | | Load | **10.5 on a 12-core machine while other builds ran** (uptime 17:39): every figure carries that noise, the single-core rows least. Re-run when the machine is quiet before quoting a ratio. | | Pin note | The pymatgen environment runs pymatgen-core **2026.10.16** under a `pymatgen` 2026.9.24 metapackage, and every fixture that records pymatgen-core records 2026.10.16 (`tools/pins/requirements/pymatgen.txt`). Nothing here compares numbers, only time | -| 0.9.1 | The `symmetry_detection` and `cif_import` rows above are the 2026-10-21 measurement of 0.9.0. 0.9.1 replaced the closure check that took 95 % of them (see `docs/open-items.md`); measured in-process on the same `al_super2.cif` (machine load 3.6): `cif_import` typical median 67 ms (min 67, max 77), against 297 ms (one run at load 9) before the change and the table's 351 ms. Not yet re-run through `run.sh` | +| 0.9.1 | The `symmetry_detection` and `cif_import` rows above are the 2026-10-21 measurement of 0.9.0. 0.9.1 replaced the closure check that took 95 % of them (see `docs/parity.md`, known limits); measured in-process on the same `al_super2.cif` (machine load 3.6): `cif_import` typical median 67 ms (min 67, max 77), against 297 ms (one run at load 9) before the change and the table's 351 ms. Not yet re-run through `run.sh` | Re-run from a fresh clone of the kit, on a quiet Mac: `gen_inputs.py` writes the shared inputs, `run.sh` builds the benchmark package (`tools/benchmarks`, release) and prints the kit's JSON lines, `bench_python.py` prints the Python lines, and `merge.py` @@ -74,7 +74,7 @@ The raw lines of the run below are `tools/benchmarks/results/2026-10-21.jsonl`. - `symmetry_detection` and `cif_import`: the kit's `CIFImport.read` always detects the symmetry of the cell it reads; pymatgen's `CifParser` only parses and `SpacegroupAnalyzer` is spglib in C. On a 32-atom P1 cell (an fcc supercell with 1 536 operations) the kit took 361 ms against 13 ms, **28 times slower**; on a four-atom cell they are level. The cause was the - closure check, not the symmetry search (`docs/open-items.md`); 0.9.1 made the check about five times faster, the rest stays listed there. + closure check, not the symmetry search (`docs/parity.md`, known limits); 0.9.1 made the check about five times faster, the rest stays listed there. - `orientation_reduce`: the kit reduces one matrix at a time (24 proper operators); orix reduces a vectorised batch (`Orientation.from_matrix` + `map_into_symmetry_reduced_zone`). The kit is 2.5 to 12 times faster at these sizes. - `ellipse_fit_1d`: level with py4DSTEM's `fit_ellipse_1D` (0.94 and 1.1). @@ -88,7 +88,7 @@ The raw lines of the run below are `tools/benchmarks/results/2026-10-21.jsonl`. | Function | Backend | Why / break-even | |---|---|---| -| `niggli_reduce`, `reflections`, `kinematic_reflections`, `symmetry_detection`, `cif_import` | CPU only | small arrays, branchy integer logic, readers; the kit is already faster than Python except where `docs/open-items.md` lists it | +| `niggli_reduce`, `reflections`, `kinematic_reflections`, `symmetry_detection`, `cif_import` | CPU only | small arrays, branchy integer logic, readers; the kit is already faster than Python except where `docs/parity.md` (known limits) lists it | | `orientation_reduce` | CPU only | 24 small matrix products per rotation; the batch (100 000 in 0.2 s) is bound by memory traffic, a GPU would pay the transfer for no gain at these sizes | | `ellipse_fit_1d` | CPU only | one 1D least-squares fit per pattern; level with Python | | `acom_library` | CPU only | built once per phase (60 ms for 600 axes), not per pattern | diff --git a/docs/open-items.md b/docs/open-items.md deleted file mode 100644 index b5b5a17..0000000 --- a/docs/open-items.md +++ /dev/null @@ -1,57 +0,0 @@ -# Open items - -A plain list, worst first. Each line: the item, the cause where it is known, what would close it. - -- **`cif_import` is 27 times slower than pymatgen's `CifParser` on a 32-atom P1 supercell (about 67 ms against 2.5 ms; 351 ms - against 2.5 ms in 0.9.0's `docs/benchmarks.md`), and `symmetry_detection` 5 times slower than spglib on the same cell (13 ms).** Not - like for like: the kit's `CIFImport.read` also detects the symmetry; pymatgen's parser only parses. Cause, MEASURED on - `tools/benchmarks/inputs/al_super2.cif` (1 536 operations, 0.9.0, in-process, median of runs): `read` 297 ms, of which - `SymmetryDetection.detect` 287 ms, of which `closedSubset` alone 281 ms (the closure check, 95 %); the search for the operations - and the parse are about 16 ms together. Not the conventional-cell step and not the search mode. The check composed every - operation with every other (2.4 million pairs), each product built as a heap-allocated matrix and searched among the 32 - translations of its rotation. 0.9.1 (same operations, same tolerance semantics, `ClosureEquivalenceTests` against the old - code): rotation ids and a product table, translations in a bucket list; `closedSubset` 58 ms, `read` 67 ms. What would refute - the cause: a profile with `closedSubset` stubbed out would still show a large `read`; it does not (the rest is about 10 ms). - Still open: 58 ms of closure is 2.4 million table and bucket lookups. Close with a generator-based closure (check the product - of each operation with a generating set, not with every operation), which needs an argument that it drops the same operations - on the pseudo-symmetric sets the split-atom tests plant; or skip the check when the file's loop already lists a closed set. -- **`reflections(kMax:)` is 2.3 to 6.6 times slower than `get_points_in_sphere`.** Not like for like: the kit also gives d, the - absence flag, the rings and their orbits. Close by timing the kit's own enumeration without the ring grouping if it matters. -- **Phase matching.** Defaults unchanged. (a) `minimumMatrixEvidence` (new, off) keeps vacuum and stray-peak positions out of the - matrix class; (b) `completenessAwareCrossPhaseRanking` stays off: it loses at small noise and wins near the pair radius - (`PhaseDiagnosisTests`). Both need one labelled scan to be switched on: a study scan with the phases marked. -- **ACOM.** 7 of 200 off-grid self-recoveries land 36 or 60 degrees away. Cause, measured: each is a coincidence twin of the - truth (60 degrees about a <111> near <112> beams, 36.87 degrees about a <100> near <013> beams; the axis lies in the zone - plane), whose kinematic pattern overlaps the truth's 0.95 to 1.0000; only the Ewald curvature term separates them, and - py4DSTEM picks the twin in 7 of the 14 truth and twin patterns (`theLargeFailuresAreCoincidenceTwinsOfTheTruth`). Inherent - to spot patterns at 200 kV, not a matcher defect; a consumer can flag it by scoring the twin of the winner. "Zone axis misses up to 12.8 degrees beyond the bank sampling" (mac4DSTEM's note) - is not reproduced here: within 2.9 degrees of a template the nearest-bank gap is at most 1.6 degrees at 600 axes; a pattern - between templates in a sparse corner of the zone is untested. Metal is not faster than all CPU cores up to 1 024 patterns - at 600 axes; 16 384 patterns and 2 400 axes are not timed yet. -- **CIF symmetry.** A Hall symbol is read and a listed operation list is checked for its count, the cell metric, closure and - the IT number's crystal class. Still unchecked: a closed list of the right class and count with the wrong translations - (P4/m listed for P4_2/m, or another origin) when the file states no Hall symbol, since the kit has no table from a - number or Hermann-Mauguin symbol to operations; and a full loop that disagrees with the Hall symbol beside it (the loop - is used, as pymatgen does). -- **Beam centre for a blocked beam.** `FriedelCentre.find(pattern:mask:)` finds it (py4DSTEM `get_origin_friedel`, upsample - factor 1, bitwise equal to the reference on six synthetic stop patterns; within 0.09 px of the planted centre without a mask, - 0.013 px with it; `validation: "none"`). `PatternCentre.find` refuses a blocked beam: its brightest region is accepted only - when it lies within half its radius (at least 2 px) of the Friedel centre, so a Bragg spot or the stop rim is no longer - returned (`theBeamFinderRefusesABlockedBeam`). Left: a two-beam pattern with one reflection stronger than about half the beam - and a weak opposite may be refused although the beam is visible; and upsample factors above 1 in `FriedelCentre`, if a - consumer needs sub-pixel peaks. -- **Ring fit.** Speed: `fitRing` at R 600 px with a band of ±90 px on 2048² took 24 s (py4DSTEM 13 s on one thread) because the - profile optimiser ran to its 80-pass cap on a start stuck at the ring-width floor and after converging; with the two stops of the - ring-fit speed fix it takes about 5 s, the same radius. Left: about 145 ms a pass at 680 000 pixels (the finite-difference Jacobian - and the normal matrix: Accelerate, or a coarse-then-fine annulus). Neighbouring ring and small rings: one cause, the tail of the - neighbouring ring inside the annulus; the bias follows the gap to the neighbour in ring sigmas (Au {111} with sigma 1.2 px: 0.03 to - 0.08 px at R 17 to 34 px, nothing from R 68), a single ring is exact at every radius, and py4DSTEM's `fit_ellipse_amorphous_ring` - has the same bias in size (`RingFitNeighbourTailTests`). Shared by the reference, so not changed. Close with a profile model that - has a term for the neighbouring ring (a new method, not a port), or keep the annulus inside the gap. -- **orix API gaps.** The orientation (Rodrigues) fundamental zone, and `dot` / `dot_outer` / `get_distance_matrix` across two - different groups (`from_euler`, `dot`, `dot_outer` and `get_distance_matrix` for one group are held). orix's - `Misorientation.reduce()` misses the smallest angle when the two groups' product set is not a group (432 with 622, 23 with - 3): it builds its zone from start . end while its equivalents need end . start (cause and pins in `Misorientation.swift`); - the kit returns the smallest angle. Not reported upstream yet. -- **Not yet used by a known consumer** (0.10.0): `PatternCentre.find`, `PatternRadialProfile.about`, `LinearScale.fit` and - `Misorientation`. Revisit at 0.12. diff --git a/docs/parity.md b/docs/parity.md index d4ae67b..5ba31b1 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -251,7 +251,7 @@ matching and phase mapping of DiffractoOrientation, as a pinned f050d207 environ | Python function or option group | DiffractoKit symbol | State | Plan | Upstream tolerance (test file) | How checked | |---|---|---|---|---|---| | `process.diffraction.crystal_phase.CrystalPhase` (`quantify_single_pattern`, `quantify_phase`: phase weights by non-negative least squares on orientation-matched Bragg peaks) | `PhaseVectorMatcher.map`, `classify`, `classifyKnownVariants` (vector matching after Thronsen et al. 2024), `PhaseReferenceLibrary`, `PhaseVectorSettings` | not held | study | tests not shipped; assert_allclose defaults | a different method; the kit's verdicts are checked against the apps' code (P16) and planted truth, not against py4DSTEM. Roadmap item 1 (diagnosed in 0.9.0): `minimumVectors` = 2 makes "matrix" by exclusion and the cross-phase winner ignores completeness; the opt-in `minimumMatrixEvidence` and `completenessAwareCrossPhaseRanking` are in the rows below | -| (no py4DSTEM counterpart) "the matrix, by exclusion" needing evidence: `CrystalPhase` has no matrix class | `PhaseVectorSettings.minimumMatrixEvidence` (default 0, off; with n > 0 the verdict "matrix" by exclusion needs at least n vectors removed as the matrix's, else "not indexed"; both classification rules) | **partly** | study | none upstream; verdicts are exact | no Python output exists. `PhaseVectorMatchingTests.matrixByExclusionCanRequireTheMatrixToHaveBeenSeen` (constructed plants: one stray peak is the shipped matrix verdict, `.notIndexed` with the setting at 1, a pattern the matrix explains stays matrix; both rules) and `ScanInputTests` (default 0). Whether it helps on real scans needs a labelled study scan (`docs/open-items.md`). Added 0.9.0 | +| (no py4DSTEM counterpart) "the matrix, by exclusion" needing evidence: `CrystalPhase` has no matrix class | `PhaseVectorSettings.minimumMatrixEvidence` (default 0, off; with n > 0 the verdict "matrix" by exclusion needs at least n vectors removed as the matrix's, else "not indexed"; both classification rules) | **partly** | study | none upstream; verdicts are exact | no Python output exists. `PhaseVectorMatchingTests.matrixByExclusionCanRequireTheMatrixToHaveBeenSeen` (constructed plants: one stray peak is the shipped matrix verdict, `.notIndexed` with the setting at 1, a pattern the matrix explains stays matrix; both rules) and `ScanInputTests` (default 0). Whether it helps on real scans needs a labelled study scan (`docs/parity.md`, known limits). Added 0.9.0 | | `Crystal.calibrate_pixel_size`, `calibrate_unit_cell`, `calculate_bragg_peak_histogram` (multi-peak fit of the pixel size or the unit cell to a Bragg-peak histogram) | `RingCalibration.calibrate(radius:d:highTensionVolts:pixelPitchMM:)` (one ring of known d); `LinearScale.estimate` (innermost reflection over many positions) | not held | study | tests not shipped; function arguments `k_step=0.002`, `k_broadening=0.002` (pixel size) and `0.005`, `0.02` (unit cell) | the kit has the single-ring closed form and the linear-scale estimate, not py4DSTEM's multi-peak fit | ### 2.5 Calibration (`process.calibration`, `data.Calibration`) @@ -396,3 +396,39 @@ Study rows that are only **partly** held and carry a ROADMAP ask (not counted ab ROADMAP item 4 also names `RingIndexing` accepted as a likelihood ratio against family membership ("document or fix"); it has no upstream counterpart and therefore no row above. + +## Known limits and decisions + +Open work is in the [issues](https://github.com/mac4DSTEM/DiffractoKit/issues), including the ones closed as *not planned* +and parked for later (`cif_import` speed #5, `reflections(kMax:)` speed #6, phase-matching options #7, orix gaps #13). Speed +against Python is in `docs/benchmarks.md`. + +- **`cif_import` and `symmetry_detection` speed.** Not like for like: `CIFImport.read` also detects the symmetry, pymatgen's + parser only parses. Measured on `tools/benchmarks/inputs/al_super2.cif` (1 536 operations): 95 % of `read` was the closure + check (`closedSubset`); 0.9.1 replaced it with rotation ids, a product table and translation buckets (`closedSubset` 58 ms, + `read` 67 ms, `ClosureEquivalenceTests` against the old code). The rest is 2.4 million lookups; a generator-based closure + would need an argument that it drops the same operations on the pseudo-symmetric sets the split-atom tests plant. +- **`reflections(kMax:)`** is 2.3 to 6.6 times slower than `get_points_in_sphere`; not like for like (the kit also gives d, + the absence flag, the rings and their orbits). +- **Phase matching defaults** are unchanged: `minimumMatrixEvidence` (off) keeps vacuum and stray-peak positions out of the + matrix class; `completenessAwareCrossPhaseRanking` (off) loses at small noise and wins near the pair radius + (`PhaseDiagnosisTests`). Both need one labelled scan to be switched on. +- **ACOM coincidence twins.** 7 of 200 off-grid self-recoveries land 36 or 60 degrees away: each is a coincidence twin of the + truth whose kinematic pattern overlaps the truth's 0.95 to 1.0000; only the Ewald curvature term separates them, and + py4DSTEM picks the twin in 7 of the 14 truth and twin patterns (`theLargeFailuresAreCoincidenceTwinsOfTheTruth`). Inherent + to spot patterns at 200 kV; a consumer can flag it by scoring the twin of the winner. Within 2.9 degrees of a template the + nearest-bank gap is at most 1.6 degrees at 600 axes; a pattern between templates in a sparse corner of the zone is untested. + Metal is not faster than all CPU cores up to 1 024 patterns at 600 axes. +- **CIF symmetry.** A Hall symbol is read, and a listed operation list is checked for its count, the cell metric, closure and + the IT number's crystal class. Unchecked: a closed list of the right class and count with the wrong translations (P4/m + listed for P4_2/m, or another origin) when the file states no Hall symbol, since the kit has no table from a number or + Hermann-Mauguin symbol to operations. +- **Beam centre for a blocked beam.** `FriedelCentre.find(pattern:mask:)` is py4DSTEM's `get_origin_friedel` at upsample + factor 1; `PatternCentre.find` refuses a blocked beam. A two-beam pattern with one reflection stronger than about half the + beam and a weak opposite may be refused although the beam is visible. +- **Ring fit.** About 5 s at R 600 px with a ±90 px band on 2048² (py4DSTEM 13 s on one thread), about 145 ms a pass at + 680 000 pixels. A neighbouring ring's tail inside the annulus biases the radius by the gap in ring sigmas (Au {111}, + sigma 1.2 px: 0.03 to 0.08 px at R 17 to 34 px, nothing from R 68); py4DSTEM's `fit_ellipse_amorphous_ring` has the same + bias (`RingFitNeighbourTailTests`), so it is not changed. Keep the annulus inside the gap. +- **orix `Misorientation.reduce()`** misses the smallest angle when the two groups' product set is not a group (432 with 622, + 23 with 3); the kit returns the smallest angle (cause and pins in `Misorientation.swift`). Not reported upstream yet. diff --git a/docs/status.md b/docs/status.md index 2786998..2905c85 100644 --- a/docs/status.md +++ b/docs/status.md @@ -17,5 +17,5 @@ Live: Tests: `DiffractoKitParityTests` holds the pinned comparisons against the Python references; the other test targets hold the fast unit tests; `DiffractoKitTestSupport` holds the helpers both use. -What is checked against what: `docs/parity.md`. Timings against Python: `docs/benchmarks.md`. What is open: -`docs/open-items.md`. Where it is going: `ROADMAP.md`. +What is checked against what: `docs/parity.md`. Timings against Python: `docs/benchmarks.md`. What is open: the GitHub +issues; known limits: `docs/parity.md`. Where it is going: `ROADMAP.md`.