diff --git a/docs/dev/adrs/accepted/space-group-database.md b/docs/dev/adrs/accepted/space-group-database.md index 74083fd76..fdaf8e1f1 100644 --- a/docs/dev/adrs/accepted/space-group-database.md +++ b/docs/dev/adrs/accepted/space-group-database.md @@ -8,10 +8,10 @@ Structure model. > This ADR follows [`AGENTS.md`](../../../../AGENTS.md). It was a > prerequisite for -> [`wyckoff-letter-detection.md`](../suggestions/wyckoff-letter-detection.md): -> Wyckoff detection can only resolve letters for space groups present in -> the bundled table, which this ADR's implementation completed for all -> 230 groups. +> [`wyckoff-letter-detection.md`](wyckoff-letter-detection.md): Wyckoff +> detection can only resolve letters for space groups present in the +> bundled table, which this ADR's implementation completed for all 230 +> groups. ## Context @@ -103,8 +103,7 @@ Coordinates and operators stay **strings** (e.g. `'(x,1/2,0)'`, `sympify`) in `crystallography.py` and to keep the file JSON-native (§2). Triclinic no-setting groups keep the `None` coordinate code, as today (see the `''`→`None` normalisation in -[`wyckoff-letter-detection.md`](../suggestions/wyckoff-letter-detection.md) -§2). +[`wyckoff-letter-detection.md`](wyckoff-letter-detection.md) §2). **Query surface preserved.** On disk the JSON is a list of setting records, each carrying the canonical `IT_number` and @@ -286,10 +285,9 @@ coordinate-system code": EasyDiffraction's `SpaceGroup` category uses the empty string `''`, while the table key uses `None`. The database keeps `(1, None)` and `(2, None)`; callers normalise `''` to `None` at lookup boundaries, as specified in -[`wyckoff-letter-detection.md`](../suggestions/wyckoff-letter-detection.md). -This is the least surprising solution because it keeps "no setting" -distinct from any real coordinate-code string without inventing a -sentinel value. +[`wyckoff-letter-detection.md`](wyckoff-letter-detection.md). This is +the least surprising solution because it keeps "no setting" distinct +from any real coordinate-code string without inventing a sentinel value. ### 8. The database file is generated, not hand-edited @@ -297,6 +295,36 @@ sentinel value. through the curation overrides and a regeneration run, keeping the file and the documented decisions in sync. +### 9. Canonical ITA `coords_xyz` (no operator form) + +Every Wyckoff `coords_xyz` template is stored in **canonical +International Tables parametric form** — each component a signed single +free variable (or an integer-coefficient combination such as `x-y`) plus +an optional rational constant, never a fractional coefficient on a +variable. cctbx's `unique_ops().as_xyz()` (the generator's raw output) +emits **operator form** (e.g. `1/2*x-1/2*y`) for coupled special +positions, which silently breaks +`crystallography._fract_constrained_flags` so a refined special-position +coordinate drifts off its symmetry site. A Wyckoff orbit is a list of +**distinct point-functions** (one per symmetry-equivalent site), and the +DB stores the **full** (centered) orbit — `coords_xyz` length equals the +ITA multiplicity. cryspy lists only the **primitive** orbit, so the full +orbit is built by **expanding** each cryspy primitive element over the +group's centering translations (the identity-rotation symops): +`full = {primitive element + centering vector}`, yielding exactly +`multiplicity` distinct canonical templates. Every replacement is +verified **exactly**: `len == multiplicity` and +`len(set) == multiplicity` (a proper full orbit with distinct elements — +no collapsed duplicates); no operator form and no fractional +coefficient; the representative's `_fract_constrained_flags` free-axis +count equals the manifold's rank (rejecting non-minimal spellings such +as `(x-y,-x+y,z)`); and parametrization-independent geometric +equivalence to the cctbx orbit (every element on a cctbx manifold, every +cctbx manifold covered). The no-operator-form and no-duplicate +invariants are enforced by the post-process before it writes, in the +unit tests, and (for operator form) by `tools/check_packaged_db.py`, +which rejects any operator-form template in the packaged wheel. + ## Consequences ### Positive @@ -346,9 +374,9 @@ and the documented decisions in sync. early when `coord_code is None` and `_get_general_position_ops()` indexes the raw key, so they need the `''`→`None` normalisation defined in - [`wyckoff-letter-detection.md`](../suggestions/wyckoff-letter-detection.md) - §2 (which also updates these call sites). This ADR delivers the data; - that ADR delivers the `None`-code consumer handling. + [`wyckoff-letter-detection.md`](wyckoff-letter-detection.md) §2 (which + also updates these call sites). This ADR delivers the data; that ADR + delivers the `None`-code consumer handling. ## Alternatives Considered @@ -409,6 +437,32 @@ pixi exec --spec cctbx --spec gemmi --spec sympy --spec pyyaml \ --print-summary ``` +**Canonical-`coords_xyz` correction (§9) — mandatory second stage.** The +rebuild has **two mandatory stages**, and the generator run above is +only the first. The generator emits cctbx operator-form `coords_xyz` for +coupled special positions and **cannot** emit canonical form itself: +cctbx always produces operator form, and the canonical ITA spelling +lives in cryspy's `wyckoff.dat`. The generator alone therefore does +**not** produce a shippable database. It **must** be followed by the +canonicalization post-process, which is the **invariant-enforcing step** +— it re-sources canonical ITA `coords_xyz` from cryspy's `wyckoff.dat` +and refuses to write unless every template is canonical (no operator +form, no fractional coefficient): + +```bash +python tmp/space-groups/helper-tools/canonicalize_coords.py --write +``` + +It rebuilds each of the 288 coupled positions' **full** orbit by +**expanding** the cryspy primitive orbit over the group's centering +translations — `coords_xyz` length equals the multiplicity and every +element is distinct. It changes only `coords_xyz`, verifies every +replacement **exactly** (distinct full orbit, canonical spelling, a +`_fract_constrained_flags` rank check, and parametrization-independent +geometric equivalence to the cctbx orbit), and asserts no operator-form +and no duplicate template remains. The `space_groups.json.gz` SHA-256 +below is **after** this correction. + Build environment: - **cctbx** from conda-forge: @@ -423,10 +477,14 @@ Build environment: Generated and curation artifacts: -- `src/easydiffraction/crystallography/space_groups.json.gz`: - `30f0051c669712ab34d991e60223c5e29264fc033b2ab03392cc01465ceba926` +- `src/easydiffraction/crystallography/space_groups.json.gz` (after the + §9 canonical-`coords_xyz` correction): + `390f0e9d0ebe27a52ee5680a1bc686123ba84c8751302fed4dee4dfaf7edf7b4` - `tmp/space-groups/helper-tools/generate_space_groups.py`: - `bf10dcfbcf9e60485037ddabc65425e61f746ad9649cd3ccc67376dd6aae241a` + `3aa5f03cd1a69bdfe0a280158c9343b65d5eaa4d75a6d58f2606fb5fbe3df83d` +- `tmp/space-groups/helper-tools/canonicalize_coords.py` (§9 + canonical-`coords_xyz` post-process): + `8f2e94b130481d2a11de057fe200d8e5fd5d3d5eec7cdd39f5d4afd13f5cb8f2` - `docs/dev/adrs/accepted/space-group-database/space_groups_overrides.yaml`: `7077eec25d0f3b852dd7096a24dc7ac438467f9cb594f91a65ce10cda0e0722a` - `tmp/space-groups/extracted-comparison/disagreements.md`: @@ -511,8 +569,8 @@ respectively. ## Related ADRs -- [`wyckoff-letter-detection.md`](../suggestions/wyckoff-letter-detection.md) - — the dependent feature; its `''`→`None` coordinate-code normalisation - and its "unsupported group" handling both build on this database. +- [`wyckoff-letter-detection.md`](wyckoff-letter-detection.md) — the + dependent feature; its `''`→`None` coordinate-code normalisation and + its "unsupported group" handling both build on this database. - [`iucr-cif-tag-alignment.md`](../accepted/iucr-cif-tag-alignment.md) — consumes space-group and Wyckoff data on export. diff --git a/docs/dev/adrs/suggestions/wyckoff-letter-detection.md b/docs/dev/adrs/accepted/wyckoff-letter-detection.md similarity index 99% rename from docs/dev/adrs/suggestions/wyckoff-letter-detection.md rename to docs/dev/adrs/accepted/wyckoff-letter-detection.md index a838598e1..47df620bc 100644 --- a/docs/dev/adrs/suggestions/wyckoff-letter-detection.md +++ b/docs/dev/adrs/accepted/wyckoff-letter-detection.md @@ -1,6 +1,6 @@ # ADR: Automatic Wyckoff Position Detection -**Status:** Proposed **Date:** 2026-06-01 +**Status:** Accepted **Date:** 2026-06-01 ## Group diff --git a/docs/dev/adrs/index.md b/docs/dev/adrs/index.md index f14cb0be0..eea0d1af1 100644 --- a/docs/dev/adrs/index.md +++ b/docs/dev/adrs/index.md @@ -46,7 +46,7 @@ folders. | Quality | Accepted | Lint Complexity Thresholds | Treats ruff PLR complexity limits as design guardrails that should not be bypassed. | [`lint-complexity-thresholds.md`](accepted/lint-complexity-thresholds.md) | | Quality | Accepted | Test Strategy | Defines layered unit, functional, integration, script, and notebook testing. | [`test-strategy.md`](accepted/test-strategy.md) | | Structure model | Accepted | Type-Neutral ADP Parameters | Keeps ADP parameter object identities stable across B/U and iso/ani switches. | [`type-neutral-adp-parameters.md`](accepted/type-neutral-adp-parameters.md) | -| Structure model | Suggestion | Automatic Wyckoff Position Detection | Detects Wyckoff letter, multiplicity, and site symmetry from space group and coordinates; calculators consume them. | [`wyckoff-letter-detection.md`](suggestions/wyckoff-letter-detection.md) | +| Structure model | Accepted | Automatic Wyckoff Position Detection | Detects Wyckoff letter, multiplicity, and site symmetry from space group and coordinates; calculators consume them. | [`wyckoff-letter-detection.md`](accepted/wyckoff-letter-detection.md) | | Structure model | Accepted | Complete Space-Group Reference Database | One-time build of a complete space_groups.json.gz (all 230 groups) from cctbx, verified against multiple sources. | [`space-group-database.md`](accepted/space-group-database.md) | | User-facing API | Accepted | Crystal Structure 3D Visualization | Adds a renderer-neutral scene model drawn by ASCII and interactive Three.js engines for viewing crystal structures. | [`crysview-structure-visualization.md`](accepted/crysview-structure-visualization.md) | | User-facing API | Accepted | Display UX Facade | Defines `project.display` and `project.rendering` responsibilities and display method names. | [`display-ux.md`](accepted/display-ux.md) | diff --git a/docs/dev/issues/closed.md b/docs/dev/issues/closed.md index c3574c2f2..e6b000055 100644 --- a/docs/dev/issues/closed.md +++ b/docs/dev/issues/closed.md @@ -4,6 +4,17 @@ Issues that have been fully resolved. Kept for historical reference. --- +## 51. Access Space Group from `AtomSites` for Wyckoff Letters + +Closed by the Wyckoff-letter-detection implementation. `AtomSite` now +derives its allowed Wyckoff letters from the parent structure's space +group (via `_resolve_structure_space_group`) instead of a hardcoded +list, and the missing-letter case is handled explicitly: untabulated +space groups leave the Wyckoff letter and multiplicity unset, while +tabulated groups detect and fill them during the update flow. + +--- + ## 103. Make `_sync_engine_from_minimizer_category` Skip-Keys Declarative Closed by the emcee minimizer implementation. Minimizer categories now diff --git a/docs/dev/issues/open.md b/docs/dev/issues/open.md index d1d400759..89434ef8f 100644 --- a/docs/dev/issues/open.md +++ b/docs/dev/issues/open.md @@ -996,24 +996,6 @@ generation. --- -## 51. 🟢 Access Space Group from `AtomSites` for Wyckoff Letters - -**Type:** Design - -`AtomSite` needs the current space group to determine allowed Wyckoff -letters but currently returns a hardcoded list. Also, a missing Wyckoff -letter case needs a decision. - -**TODOs:** - -- [default.py](src/easydiffraction/datablocks/structure/categories/atom_sites/default.py#L163) -- [default.py](src/easydiffraction/datablocks/structure/categories/atom_sites/default.py#L179) -- [default.py](src/easydiffraction/datablocks/structure/categories/atom_sites/default.py#L353) - -**Depends on:** nothing. - ---- - ## 52. 🟢 Rename Line-Segment Background `y` to `intensity` **Type:** Naming @@ -1951,7 +1933,6 @@ only render the index column when no explicit id column is present. | 48 | Fix CrysPy TOF instrument default | 🟢 Low | Bug workaround | | 49 | Automate space group CIF name variants | 🟢 Low | Maintainability | | 50 | Clarify `Cell._update` minimizer param | 🟢 Low | Cleanup | -| 51 | Access space group for Wyckoff letters | 🟢 Low | Design | | 52 | Rename line-segment `y` to `intensity` | 🟢 Low | Naming | | 53 | Move `show()` to `CategoryCollection` | 🟢 Low | Maintainability | | 54 | Add `point_id` to excluded regions | 🟢 Low | Completeness | diff --git a/docs/dev/package-structure/full.md b/docs/dev/package-structure/full.md index cad676efe..de4e69ac5 100644 --- a/docs/dev/package-structure/full.md +++ b/docs/dev/package-structure/full.md @@ -244,6 +244,7 @@ │ │ ├── 🏷️ class TypeValidator │ │ ├── 🏷️ class RangeValidator │ │ ├── 🏷️ class MembershipValidator +│ │ ├── 🏷️ class PermissiveMembershipValidator │ │ ├── 🏷️ class RegexValidator │ │ └── 🏷️ class AttributeSpec │ └── 📄 variable.py @@ -262,6 +263,7 @@ ├── 📁 crystallography │ ├── 📄 __init__.py │ ├── 📄 crystallography.py +│ │ └── 🏷️ class WyckoffPosition │ └── 📄 space_groups.py ├── 📁 datablocks │ ├── 📁 experiment @@ -464,6 +466,13 @@ │ │ │ │ │ └── 🏷️ class SpaceGroup │ │ │ │ └── 📄 factory.py │ │ │ │ └── 🏷️ class SpaceGroupFactory +│ │ │ ├── 📁 space_group_wyckoff +│ │ │ │ ├── 📄 __init__.py +│ │ │ │ ├── 📄 default.py +│ │ │ │ │ ├── 🏷️ class SpaceGroupWyckoff +│ │ │ │ │ └── 🏷️ class SpaceGroupWyckoffCollection +│ │ │ │ └── 📄 factory.py +│ │ │ │ └── 🏷️ class SpaceGroupWyckoffFactory │ │ │ └── 📄 __init__.py │ │ ├── 📁 item │ │ │ ├── 📄 __init__.py diff --git a/docs/dev/package-structure/short.md b/docs/dev/package-structure/short.md index 23c0e68b8..61c705dd4 100644 --- a/docs/dev/package-structure/short.md +++ b/docs/dev/package-structure/short.md @@ -222,6 +222,10 @@ │ │ │ │ ├── 📄 __init__.py │ │ │ │ ├── 📄 default.py │ │ │ │ └── 📄 factory.py +│ │ │ ├── 📁 space_group_wyckoff +│ │ │ │ ├── 📄 __init__.py +│ │ │ │ ├── 📄 default.py +│ │ │ │ └── 📄 factory.py │ │ │ └── 📄 __init__.py │ │ ├── 📁 item │ │ │ ├── 📄 __init__.py diff --git a/docs/dev/plans/space-group-database.md b/docs/dev/plans/space-group-database.md index fe2a4b93f..814b36e0b 100644 --- a/docs/dev/plans/space-group-database.md +++ b/docs/dev/plans/space-group-database.md @@ -26,7 +26,7 @@ This plan owns the ADR [`docs/dev/adrs/accepted/space-group-database.md`](../adrs/accepted/space-group-database.md) (drafted via `/draft-adr`, review cycle closed). It is a **prerequisite** for -[`wyckoff-letter-detection`](../adrs/suggestions/wyckoff-letter-detection.md): +[`wyckoff-letter-detection`](../adrs/accepted/wyckoff-letter-detection.md): this plan delivers the complete data; that feature delivers the `''`→`None` consumer handling so the triclinic groups use it. @@ -272,3 +272,253 @@ flagged rows visible for later International Tables verification. The data now ships as transparent, inspectable JSON instead of an opaque binary pickle. Existing projects load unchanged; structures in the previously-missing groups now get correct symmetry handling. + +--- + +## Follow-up phase: canonical Wyckoff `coords_xyz` templates + +_Added after #187 shipped — a second, standalone cycle of this ADR's +implementation. It is the **prerequisite** that +[`wyckoff-letter-detection`](../adrs/accepted/wyckoff-letter-detection.md) +§10 / Decision 15 depends on and gates on at its P1.0, and it ships on +its **own PR** (separate from #187). When implementing this phase, treat +the `CT` checklist below as the active Phase 1 steps — the #187 Phase 1 +above is complete._ + +### Status (this phase) + +- [x] Phase 1 — Implementation (data + tooling + docs) +- [x] Phase 1 review gate +- [x] Phase 2 — Verification (tests + checks) + +### Problem + +The bundled `space_groups.json.gz` stores cctbx **operator-form** +`coords_xyz` — e.g. R-3m (IT 166) `h` is `(1/2*x-1/2*y,-1/2*x+1/2*y,z)` +— for the coupled special positions (verified: 3826 templates; per the +wyckoff plan's Decision 15, 288 positions across 117 IT numbers). +`_fract_constrained_flags()` (`crystallography.py:221`) decides which +axis is constrained purely by **which variable symbol is absent** from +the representative: canonical `(x,-x,z)` has `y` absent → `fract_y` +constrained → slaved to `-x`; but operator-form `(1/2*x-1/2*y,…)` makes +**both `x` and `y` appear** → `fract_y` is wrongly "free" and the `y=-x` +coupling is lost, so a refined special-position coordinate drifts off +its symmetry site (the `ed-6` fit-3 → fit-4 regression). +`_apply_fract_constraints()` (`:250`) and `_parse_rotation_matrix()` +(`:350`, which does `int(coeff_str)`) share the same expectation. The +constraint **code** is correct; only the **data** spelling is wrong. +**Root cause:** the generator emitted cctbx operator-form `coords_xyz` +for these positions instead of cryspy's canonical form; the ADR's +intended Wyckoff source, cryspy's `wyckoff.dat`, already holds the +canonical ITA form for them (verified — R-3m gives `(x,x,1/2)`, +`(x,-x,1/2)`, …). + +### Decisions (this phase) + +1. **Produce canonical `coords_xyz` via a cctbx-free post-process that + re-sources from cryspy's `wyckoff.dat`.** The root cause is that the + generator's `_extract_wyckoff_positions` built `coords_xyz` from + cctbx (`position.unique_ops().as_xyz()`, operator form) while + cryspy's `wyckoff.dat` — the ADR's intended Wyckoff source — was read + only for count-validation. A Wyckoff orbit is a list of **distinct + point-functions** (one per equivalent site), not a set of manifolds. + A local tool loads the bundled DB and, for each operator-form + position, builds the **full** canonical orbit by **expanding** the + cryspy primitive orbit over the group's centering translations (the + identity-rotation symops): each cryspy primitive element shifted by + each centering vector yields one canonical full-orbit element. This + **preserves the full (centered) orbit** — `coords_xyz` length stays + equal to the multiplicity, matching the #187 baseline — with + **distinct** elements. (cctbx stores the full centered orbit; cryspy + lists only the **primitive** orbit, with centering implicit.) + Canonical form = each component a signed single free variable (or an + integer-coefficient ITA combination such as `x-y`) plus an optional + rational constant — **no fractional coefficient on a variable** — + with each genuine free DOF reduced to one canonical variable so + dependent axes' symbols are absent. _Rejected alternatives: (a) + replacing `coords_xyz` with cryspy's orbit **directly** silently + reduces centered orbits to primitive, breaking the + length-equals-multiplicity invariant (review-1 [Finding 2]); (b) + re-spelling each cctbx element by **column space** collapses distinct + point-functions sharing a manifold into **duplicate** templates + (review-2 [Finding 1]). The centering expansion avoids both. No cctbx + re-run is needed; fixing the generator + full cctbx re-run adds a + heavy install + reproducibility risk for the same cryspy↔cctbx + reconciliation._ +2. **Verify each replacement two ways** (review-1 [P1] plus the CT1 + correctness gap): (a) **exact symbolic orbit equivalence** (`sympy`) + — the cryspy canonical orbit and the cctbx operator-form orbit + describe the same point set as rational affine forms, not merely + agree at sampled parameters; **and** (b) **constraint correctness** — + the canonical representative's free-symbol set produces the right + `_fract_constrained_flags()` (free-DOF count matches the orbit's true + dimensionality; dependent axes constrained). Orbit equivalence alone + is insufficient: a non-minimal form like `(x-y,-x+y,z)` would pass it + yet leave `fract_y` wrongly free. _Rejected alternative — a blind + `sympy` re-parametrise of the existing operator strings — risks + exactly that non-minimal failure mode and re-derives the ITA + convention cryspy already encodes._ +3. **No constraint-code change.** `_fract_constrained_flags()` / + `_apply_fract_constraints()` are correct as-is. +4. **No new dependency, no cctbx.** The post-process uses `cryspy`'s + `wyckoff.dat` + `numpy`/`sympy` (already project deps); `cctbx` is + not used at all (it stays generation-only, relevant only if the + generator is ever fully re-run). +5. **Guard the output:** the canonicalize post-process is the + invariant-enforcing step — it refuses to write unless every template + is canonical and the orbit is distinct — backed by a + `tools/check_packaged_db.py` assertion rejecting operator-form + leakage in the packaged wheel and a unit data-invariant over loaded + `SPACE_GROUPS`. (The generator stays cctbx extraction and cannot + self-assert this; see Decision 1 and CT2.) +6. **Record it in the ADR:** add the canonical-`coords_xyz` invariant as + a decision and update _Build Provenance_ with the new DB SHA-256 and + the transform's SHA-256. + +### Open questions (this phase) + +- If any affected position cannot be canonicalised automatically, fall + back to a maintainer-curated entry in the existing + `space_groups_overrides.yaml` channel and record it. Not expected. + +### Concrete files likely to change (this phase) + +- `src/easydiffraction/crystallography/space_groups.json.gz` — rewritten + with canonical `coords_xyz`; all other fields unchanged. +- `tmp/space-groups/helper-tools/canonicalize_coords.py` — **new**, + local ignored cctbx-free post-process: load DB → orbit-match each + position to cryspy's canonical orbit → verify (mod-1 orbit + membership + `_fract_constrained_flags`) → rewrite `coords_xyz` (all + other fields byte-identical). +- `tmp/space-groups/helper-tools/generate_space_groups.py` — **local, + ignored**: stays cctbx extraction; it **cannot** self-canonicalize + (cctbx always emits operator form, and the canonical spelling + checks + need the project env — see CT2). Its `_extract_wyckoff_positions` + docstring directs to the mandatory `canonicalize_coords.py` second + stage. Not re-run now. +- `docs/dev/adrs/accepted/space-group-database/space_groups_overrides.yaml` + — record any position the orbit match leaves ambiguous (curated vs + International Tables). +- `tools/check_packaged_db.py` — assert no packaged `coords_xyz` is + operator-form. +- `docs/dev/adrs/accepted/space-group-database.md` — canonical-form + invariant decision + updated _Build Provenance_. +- Phase 2 tests: + - `tests/unit/easydiffraction/crystallography/test_space_groups.py` + (or `_coverage.py`) — data invariant: no `SPACE_GROUPS` `coords_xyz` + is operator-form. + - `tests/unit/easydiffraction/crystallography/test_crystallography.py` + (or `_coverage.py`) — coupled-position constraint regression: for + R-3m `h` (`(x,-x,z)`), `_fract_constrained_flags()` marks `fract_y` + constrained and `_apply_fract_constraints()` slaves it to `-fract_x` + after a `fract_x` edit (existing tests cover only all-fixed / + all-free sites). + - `ed-6` functional/script regression: the special-position fit stays + on-site across fit-3 → fit-4. + +### Implementation steps — canonical-templates phase + +Per-step commit discipline as in Phase 1, **with the same deliberate +exception** that `tmp/space-groups/helper-tools/*` are local curation +tooling, not branch deliverables (review-1 [P1]). The local transform +and generator changes are therefore **not their own commits**: each +tracked commit stages only the tracked deliverable it produces, and the +local tools are recorded by SHA-256 in the ADR provenance (CT4). No step +commits only ignored files, and no empty commits. + +- [x] **CT1 — Canonicalise the DB via the cctbx-free post-process.** + Finish the local + `tmp/space-groups/helper-tools/canonicalize_coords.py`: parse + cryspy `wyckoff.dat`, then build each operator-form position's + **full** canonical orbit by **expanding the cryspy primitive orbit + over the group's centering translations** (`coords_xyz` length + equals the multiplicity, all elements **distinct**). Verify per + position, **exactly** — `len(set) == multiplicity` (distinct full + orbit), no operator form, no fractional coefficient, correct + `_fract_constrained_flags()` (free-axis count equals the manifold + rank), and parametrization-independent geometric equivalence to + the cctbx orbit. Curate against International Tables any case the + cryspy match leaves ambiguous, recording it in + `space_groups_overrides.yaml`. Run it to rewrite + `src/easydiffraction/crystallography/space_groups.json.gz` (R-3m + `h` → `(x,-x,z)`, 18-element full orbit; all non-`coords_xyz` + fields byte-identical). The tool is local tooling (recorded by SHA + in CT4); **this commit stages only the regenerated + `space_groups.json.gz`**. Commit: + `Canonicalize space-group coords_xyz templates` +- [x] **CT2 — Make the durable rebuild path two-stage with an + invariant-enforcing post-process (local prep, no commit).** The + generator (`generate_space_groups.py`) runs in a throwaway + cctbx-only env, and cctbx **always** emits operator-form coords + for coupled positions — so the generator cannot itself source + canonical coords or self-assert a no-operator-form invariant (the + canonical spelling lives in cryspy's `wyckoff.dat`, and the + constraint check needs `easydiffraction`, i.e. the project env). + The durable rebuild path is therefore **two mandatory stages**: + (1) the generator (cctbx env), then (2) + `canonicalize_coords.py --write` (project env), which is the + **invariant-enforcing step** — it re-sources canonical ITA coords, + verifies each exactly, and refuses to write unless every template + is canonical (no operator form, no fractional coefficient). The + generator's `_extract_wyckoff_positions` docstring directs to this + mandatory post-process and the ADR _Build Provenance_ documents + both stages (review-1 [Finding 1]). **No re-run now** (CT1 already + produced the canonical DB). Local curation tooling — not + committed; its SHA-256 is recorded in CT4. +- [x] **CT3 — Packaging assertion.** Extend the tracked + `tools/check_packaged_db.py` to assert no packaged `coords_xyz` + template is operator-form (catches future regression at the wheel + layer). Commit: `Assert canonical coords_xyz in packaged DB check` +- [x] **CT4 — ADR invariant + provenance.** Add the + canonical-`coords_xyz` invariant as a decision in + `docs/dev/adrs/accepted/space-group-database.md`, and update its + _Build Provenance_ with the new `space_groups.json.gz` SHA-256, + the `canonicalize_coords.py` transform SHA-256, **and the updated + `generate_space_groups.py` SHA-256** plus the canonical-invariant + step in the rebuild path (review-1 [P2]). Commit: + `Record canonical coords_xyz invariant and provenance` +- [x] **CT5 — Phase 1 review gate.** No code. Mark `[x]`, commit the + checklist update alone, hand off to review. Commit: + `Reach canonical-templates Phase 1 review gate` + +### Phase 2 — Verification (canonical-templates phase) + +Add the tests above, then run (zsh-safe capture): + +```bash +pixi run fix +pixi run test-structure-check > /tmp/easydiffraction-test-structure.log 2>&1; test_structure_exit_code=$?; tail -n 50 /tmp/easydiffraction-test-structure.log; exit $test_structure_exit_code +pixi run check > /tmp/easydiffraction-check.log 2>&1; check_exit_code=$?; tail -n 200 /tmp/easydiffraction-check.log; exit $check_exit_code +pixi run unit-tests > /tmp/easydiffraction-unit.log 2>&1; unit_tests_exit_code=$?; tail -n 100 /tmp/easydiffraction-unit.log; exit $unit_tests_exit_code +pixi run integration-tests > /tmp/easydiffraction-integration.log 2>&1; integration_tests_exit_code=$?; tail -n 100 /tmp/easydiffraction-integration.log; exit $integration_tests_exit_code +pixi run script-tests > /tmp/easydiffraction-script.log 2>&1; script_tests_exit_code=$?; tail -n 100 /tmp/easydiffraction-script.log; exit $script_tests_exit_code +``` + +Then the packaging regression from #187's Phase 2 (build the wheel + +`python tools/check_packaged_db.py dist/*.whl`), which now also +exercises the new operator-form assertion. + +**Verification note (for `/review-impl-2`).** Phase 2's `pixi run fix` +also reformatted docstrings in +`src/easydiffraction/display/plotters/plotly.py` and +`src/easydiffraction/report/fit_plot.py` — **pre-existing** debt from +this branch's plotting commits (`bb817a5fd`, `176e6e682`, `bec2f5e73`), +surfaced by the recently-enabled `format-docstring` hook and required +for `pixi run check` to pass. It is **unrelated** to canonical-templates +and was folded into the verification commit `92c41124b` (authored +manually); drop or move it if this branch is split from the plotting +stream. + +### Suggested Pull Request — canonical-templates fix + +**Title:** Store space-group Wyckoff coordinates in canonical form + +**Description:** A refined atom on certain special positions (sites with +linked coordinates, like `(x, -x, z)`) could drift off its symmetry +position during a fit, because the bundled space-group table stored +those coordinate templates in an internal operator form the +symmetry-constraint code didn't recognise. This change rewrites every +affected template into the standard International Tables form so the +constraints hold, adds guards so the table can't silently regress, and +fixes the related `ed-6` tutorial refinement. It also unblocks automatic +Wyckoff-position detection, which builds on these canonical templates. diff --git a/docs/dev/plans/wyckoff-letter-detection.md b/docs/dev/plans/wyckoff-letter-detection.md index 11b115ec7..2bfcb4bc5 100644 --- a/docs/dev/plans/wyckoff-letter-detection.md +++ b/docs/dev/plans/wyckoff-letter-detection.md @@ -1,20 +1,20 @@ # Plan: Automatic Wyckoff Position Detection This plan follows [`AGENTS.md`](../../../AGENTS.md) and implements the -[`wyckoff-letter-detection`](../adrs/suggestions/wyckoff-letter-detection.md) +[`wyckoff-letter-detection`](../adrs/accepted/wyckoff-letter-detection.md) ADR. No deliberate exception to `AGENTS.md` is taken. ## Status - [x] ADR review gate closed -- [ ] Phase 1 — Implementation (code + docs) -- [ ] Phase 1 review gate -- [ ] Phase 2 — Verification (tests + `pixi` checks) +- [x] Phase 1 — Implementation (code + docs) +- [x] Phase 1 review gate +- [x] Phase 2 — Verification (tests + `pixi` checks) ## ADR This plan implements the -[`wyckoff-letter-detection`](../adrs/suggestions/wyckoff-letter-detection.md) +[`wyckoff-letter-detection`](../adrs/accepted/wyckoff-letter-detection.md) ADR. Earlier ADR review cycles closed at review 10 and then review 16 (adding the derived `space_group_Wyckoff` category and space-group-key re-detection); that text was committed as `0f3bc269c` @@ -249,7 +249,7 @@ before moving to the next step or the Phase 1 review gate**, per The ADR commit + design-phase review/reply cleanup are handled by `/draft-impl-1` Phase A before P1.1. -- [ ] **P1.0 — Verify the ADR gate and the §10 prerequisite.** No code. +- [x] **P1.0 — Verify the ADR gate and the §10 prerequisite.** No code. Ensure `git branch --show-current` is `wyckoff-letter-detection`; if not, stop before editing and ask the user to switch to the target branch outside the shortcut. Confirm the ADR on disk @@ -264,7 +264,7 @@ The ADR commit + design-phase review/reply cleanup are handled by remains, **stop**: the space-group-database prerequisite must land before this plan's detection and snapping can be implemented. Commit: `Confirm wyckoff letter detection ADR gate` -- [ ] **P1.1 — Orbit matcher in the crystallography submodule.** Add to +- [x] **P1.1 — Orbit matcher in the crystallography submodule.** Add to `crystallography.py`: frozen `WyckoffPosition(letter, multiplicity, site_symmetry, coord_template)`, `_WYCKOFF_DETECTION_TOL = 1e-3`, `_normalize_coord_code()`, @@ -276,7 +276,7 @@ The ADR commit + design-phase review/reply cleanup are handled by Export new public names via `crystallography/__init__.py` if public. Commit: `Add Wyckoff orbit detection to crystallography module` -- [ ] **P1.2 — Derived `space_group_wyckoff` category.** Add the +- [x] **P1.2 — Derived `space_group_wyckoff` category.** Add the `space_group_wyckoff` package with a `SpaceGroupWyckoff` item keyed by `id` (`_space_group_Wyckoff.id`) and read-only descriptors for `id`, `letter`, `multiplicity`, `site_symmetry`, @@ -284,7 +284,7 @@ The ADR commit + design-phase review/reply cleanup are handled by mutation methods raising and a private `_replace_from_space_group` rebuild method that creates/adopts rows from `SPACE_GROUPS[key]`. Commit: `Add derived space group Wyckoff category` -- [ ] **P1.3 — Wire `space_group_wyckoff` into `Structure`.** Add it as +- [x] **P1.3 — Wire `space_group_wyckoff` into `Structure`.** Add it as a read-only sibling category on `Structure`, rebuild it when structure categories update so it tracks the current space group, keep it empty for absent groups, and exclude it from project CIF @@ -294,14 +294,14 @@ The ADR commit + design-phase review/reply cleanup are handled by reads `SPACE_GROUPS` through the crystallography helpers rather than depending on this collection. Commit: `Wire derived Wyckoff table into Structure` -- [ ] **P1.4 — Read-only multiplicity + detection mutator on +- [x] **P1.4 — Read-only multiplicity + detection mutator on `AtomSite`.** Add only `multiplicity` as a read-only derived descriptor on `AtomSite` with `CifHandler` for `_atom_site.site_symmetry_multiplicity`, empty form `None`, and no public setter. Add `_set_wyckoff_letter_detected()` modelled on `_set_value_from_minimizer`. Do not add `site_symmetry` to `AtomSite`. Commit: `Add read-only multiplicity to AtomSite` -- [ ] **P1.5 — Dynamic allowed letters + unsupported-group validation.** +- [x] **P1.5 — Dynamic allowed letters + unsupported-group validation.** Make `_wyckoff_letter_allowed_values` return `['', *list(SPACE_GROUPS[key]['Wyckoff_positions'])]` for a supported group and `[]` for an absent one. Add the @@ -314,7 +314,7 @@ The ADR commit + design-phase review/reply cleanup are handled by "needs context validation" marker instead of treating missing context as an unsupported group. Commit: `Derive allowed Wyckoff letters from the space group` -- [ ] **P1.6 — Detection triggers in the atom-site update flow.** In +- [x] **P1.6 — Detection triggers in the atom-site update flow.** In `_update(*, called_by_minimizer=False)`, implement fill-if-empty, re-detect-on-coordinate-change, and re-detect-on-space-group-key change with per-atom coordinate and `(name_hm, coord_code)` @@ -325,21 +325,55 @@ The ADR commit + design-phase review/reply cleanup are handled by stored letter. For supported keys, refresh letter, multiplicity, and selected representative; for unsupported keys, preserve stored letters as unvalidated values, set multiplicity to `None`, skip - constraints, and warn. Use the selected `coord_template` for - snapping and constrained-axis flags. Warn when coordinate or - supported space-group edits move the letter, when a user - letter-set snaps coordinates, and when a same-letter coordinate - edit snaps coordinates. Honour `called_by_minimizer=True`; - populate `multiplicity` from `wyckoff_position_info`. - Site-symmetry display data comes from + constraints, and warn. Snap by **solving the free parameters** — + least-squares project the coordinate onto the selected + `coord_template`'s manifold, then set every axis to that manifold + point — so centering copies and off-canonical-slot representatives + (e.g. 6e `(0,x,0)`) snap correctly; derive constrained-axis flags + from the same representative. This free-parameter-solving snap + **replaces the positional `_apply_fract_constraints` + substitution** (a deliberate deviation from ADR §5's "existing + constraint step", decided during P1.1; reflect it in ADR §5 at the + P1.9 promotion). Warn when coordinate or supported space-group + edits move the letter, when a user letter-set snaps coordinates, + and when a same-letter coordinate edit snaps coordinates. Honour + `called_by_minimizer=True`; populate `multiplicity` from + `wyckoff_position_info`. Site-symmetry display data comes from `structure.space_group_wyckoff`, not from `AtomSite`. Commit: `Detect and track Wyckoff letters in the update flow` -- [ ] **P1.7 — Calculator consumes model multiplicity.** Replace the + + _P1.6 implementation decisions (implemented):_ + - **Snap = slot-aware free-parameter-solving** + (`crystallography.snap_to_wyckoff_template`, already committed): + solve the free params from the **free (refinable) axes**, keep those + axes, and derive the constrained axes. **Not** manifold projection — + that averaged/moved the free axis and fought the minimizer. Handles + off-canonical reps like 6e `(0,x,0)` (keep `fract_y`, set + `fract_x=fract_z=0`); matches the old substitution for canonical + sites, so the fit is unaffected. Per-axis constraint flags are + slot-based (first-occurrence), not symbol-based. + - **Warning gating (per the chosen option):** pass + `called_by_minimizer=True` **only at the per-iteration minimizer + objective** — `analysis/fit_helpers/metrics.py:181` (residual calc; + verify `analysis/fitting.py:382` too) — and **leave** the fit-setup + (`fitting.py:209`) and flush (`analysis.py:174`) sites `False` so + detection still runs there. Gate re-detection **and** the + "adjusted"/"moved-letter" warnings on `not called_by_minimizer`, so + they never fire per fit step. + - **Remaining:** rewrite + `_apply_atomic_coordinates_symmetry_constraints` (per atom: resolve + the `_wyckoff_letter_needs_validation` marker → decide + detect/trigger → snap → set `multiplicity` + constrained flags → + refresh baselines), thread `called_by_minimizer` through + `AtomSites._update`, change the objective call site(s), then + **verify by running `test_fit_neutron_pd_cwl_hs`** and smoke tests. + +- [x] **P1.7 — Calculator consumes model multiplicity.** Replace the `SPACE_GROUPS` lookup in `cryspy._update_atom_multiplicity` with `atom_site.multiplicity.value`; when it is `None`, leave the backend's inferred multiplicity in place. Commit: `Read multiplicity from the model in the cryspy calculator` -- [ ] **P1.8 — CIF and report output.** Ensure project CIF writes +- [x] **P1.8 — CIF and report output.** Ensure project CIF writes `_atom_site.Wyckoff_symbol` and `_atom_site.site_symmetry_multiplicity` but excludes the derived `space_group_Wyckoff` loop. Ensure read ignores incoming @@ -351,14 +385,62 @@ The ADR commit + design-phase review/reply cleanup are handled by `_space_group_Wyckoff.{id,letter,multiplicity,site_symmetry,coords_xyz}` loop. Commit: `Serialize Wyckoff multiplicity and report Wyckoff table` -- [ ] **P1.9 — Promote ADR, close #51, remove stale TODOs.** `git mv` + + _P1.8 implementation decisions (implemented):_ + - **Project-CIF write of `_atom_site.site_symmetry_multiplicity`** + is already automatic: P1.4 added the `multiplicity` descriptor + with that CIF handler, and it is part of `AtomSite.parameters`, + so the atom-site loop emits it (value `?` for untabulated + sites). The `_space_group_Wyckoff` loop exclusion is already + provided by P1.3's `Structure._serializable_categories` + override. No new write-side code was needed in P1.8. + - **Read ignore of incoming `_space_group_Wyckoff.*`** is done by + a no-op `SpaceGroupWyckoffCollection.from_cif` override (the + structure read loop iterates *all* categories, including the + derived one). A hand-edited `_space_group_Wyckoff` loop is + discarded; the table is rebuilt from the space group on update. + - **Read ignore of incoming `_atom_site.site_symmetry_multiplicity`** + relies on re-derivation: the value is parsed into the + descriptor but overwritten by detection on the next + `_update_categories` (verified: file value `777` → re-derived + `1`). No extra read-side code. + - **Report `_space_group_Wyckoff.coords_xyz` = representative + coordinate only** (first orbit member, e.g. `(x,x,z)`), not the + full orbit. The collection stores the full centred orbit (up to + ~3551 chars for multiplicity-192 cubic positions), but the IUCr + report loop formatter rejects loop cells > 80 chars. Emitting + the representative keeps every space group's report valid and + matches the conventional ITA "Coordinates" entry. Decision + confirmed with the user during P1.8. The full orbit remains + available on the in-memory `space_group_wyckoff` category. + +- [x] **P1.9 — Promote ADR, close #51, remove stale TODOs.** `git mv` `wyckoff-letter-detection.md` from `suggestions/` to `accepted/`, set `**Status:** Accepted`, flip its `docs/dev/adrs/index.md` row to `Accepted`, and fix links with `git grep -n`. Move issue #51 from `open.md` to `closed.md` and delete the resolved TODOs in `default.py` (~200–211, ~225, ~569). Commit: `Promote wyckoff-letter-detection ADR and close issue #51` -- [ ] **P1.10 — Phase 1 review gate.** No code. Mark this `[x]`, commit + + _P1.9 notes (implemented):_ + - ADR moved with `git mv` to `accepted/`, `**Status:** Accepted`, + `index.md` row flipped to `Accepted` with the `accepted/` link. + - Inbound links to the old `suggestions/` path fixed in + `accepted/space-group-database.md` (5) and + `plans/space-group-database.md` (2), plus this plan's own ADR + cross-references. The ADR's `../../../../` root paths are + depth-invariant and its `../accepted/` sibling links still + resolve, so they were left unchanged (minimal diff). + - #51 moved from `open.md` (detailed section + summary-table row) + to `closed.md`. + - The `default.py` TODOs #51 referenced (old lines ~163/179/353, + about the hardcoded allowed-letter list and the missing-letter + case) were **already removed** when P1.5/P1.6 rewrote those + methods to resolve #51, so there is no `default.py` change in + this step. The only remaining TODO (label-regex/dict-key, line + ~68) is unrelated to #51 and was intentionally left. + +- [x] **P1.10 — Phase 1 review gate.** No code. Mark this `[x]`, commit the checklist update alone, then stop for the Phase 1 review. Commit: `Reach Phase 1 review gate` diff --git a/docs/docs/tutorials/ed-10.ipynb b/docs/docs/tutorials/ed-10.ipynb index e2b947d23..3815f24ea 100644 --- a/docs/docs/tutorials/ed-10.ipynb +++ b/docs/docs/tutorials/ed-10.ipynb @@ -112,7 +112,6 @@ " fract_x=0.0,\n", " fract_y=0.0,\n", " fract_z=0.0,\n", - " wyckoff_letter='a',\n", " adp_iso=0.5,\n", ")" ] diff --git a/docs/docs/tutorials/ed-10.py b/docs/docs/tutorials/ed-10.py index 1b5077fb8..751831508 100644 --- a/docs/docs/tutorials/ed-10.py +++ b/docs/docs/tutorials/ed-10.py @@ -39,7 +39,6 @@ fract_x=0.0, fract_y=0.0, fract_z=0.0, - wyckoff_letter='a', adp_iso=0.5, ) diff --git a/docs/docs/tutorials/ed-11.ipynb b/docs/docs/tutorials/ed-11.ipynb index 13f867925..b5d03cf56 100644 --- a/docs/docs/tutorials/ed-11.ipynb +++ b/docs/docs/tutorials/ed-11.ipynb @@ -139,7 +139,6 @@ " fract_x=0,\n", " fract_y=0,\n", " fract_z=0,\n", - " wyckoff_letter='a',\n", " adp_iso=0.5,\n", ")" ] diff --git a/docs/docs/tutorials/ed-11.py b/docs/docs/tutorials/ed-11.py index e8d878f27..e08e81d63 100644 --- a/docs/docs/tutorials/ed-11.py +++ b/docs/docs/tutorials/ed-11.py @@ -47,7 +47,6 @@ fract_x=0, fract_y=0, fract_z=0, - wyckoff_letter='a', adp_iso=0.5, ) diff --git a/docs/docs/tutorials/ed-12.ipynb b/docs/docs/tutorials/ed-12.ipynb index 4724b20b6..ed2c0a0b5 100644 --- a/docs/docs/tutorials/ed-12.ipynb +++ b/docs/docs/tutorials/ed-12.ipynb @@ -144,7 +144,6 @@ " fract_x=0,\n", " fract_y=0,\n", " fract_z=0,\n", - " wyckoff_letter='a',\n", " adp_iso=1.0,\n", ")\n", "project.structures['nacl'].atom_sites.create(\n", @@ -153,7 +152,6 @@ " fract_x=0.5,\n", " fract_y=0.5,\n", " fract_z=0.5,\n", - " wyckoff_letter='b',\n", " adp_iso=1.0,\n", ")" ] diff --git a/docs/docs/tutorials/ed-12.py b/docs/docs/tutorials/ed-12.py index 2e38828ea..3add79927 100644 --- a/docs/docs/tutorials/ed-12.py +++ b/docs/docs/tutorials/ed-12.py @@ -52,7 +52,6 @@ fract_x=0, fract_y=0, fract_z=0, - wyckoff_letter='a', adp_iso=1.0, ) project.structures['nacl'].atom_sites.create( @@ -61,7 +60,6 @@ fract_x=0.5, fract_y=0.5, fract_z=0.5, - wyckoff_letter='b', adp_iso=1.0, ) diff --git a/docs/docs/tutorials/ed-13.ipynb b/docs/docs/tutorials/ed-13.ipynb index 1ac483d2c..9a84c90cd 100644 --- a/docs/docs/tutorials/ed-13.ipynb +++ b/docs/docs/tutorials/ed-13.ipynb @@ -710,7 +710,7 @@ "_atom_site.occupancy\n", "_atom_site.ADP_type\n", "_atom_site.B_iso_or_equiv\n", - "Si Si 0 0 0 a 1.0 Biso 0.89\n", + "Si Si 0.125 0.125 0.125 a 1.0 Biso 0.89\n", "```" ] }, @@ -838,10 +838,9 @@ "project_1.structures['si'].atom_sites.create(\n", " label='Si',\n", " type_symbol='Si',\n", - " fract_x=0,\n", - " fract_y=0,\n", - " fract_z=0,\n", - " wyckoff_letter='a',\n", + " fract_x=0.125,\n", + " fract_y=0.125,\n", + " fract_z=0.125,\n", " adp_iso=0.89,\n", ")" ] @@ -1805,7 +1804,6 @@ " fract_x=0,\n", " fract_y=0,\n", " fract_z=0,\n", - " wyckoff_letter='a',\n", " adp_iso=0.95,\n", " occupancy=0.5,\n", ")\n", @@ -1815,7 +1813,6 @@ " fract_x=0,\n", " fract_y=0,\n", " fract_z=0,\n", - " wyckoff_letter='a',\n", " adp_iso=0.95,\n", " occupancy=0.5,\n", ")\n", @@ -1825,7 +1822,6 @@ " fract_x=0.5,\n", " fract_y=0.5,\n", " fract_z=0.5,\n", - " wyckoff_letter='b',\n", " adp_iso=0.80,\n", ")\n", "project_2.structures['lbco'].atom_sites.create(\n", @@ -1834,7 +1830,6 @@ " fract_x=0,\n", " fract_y=0.5,\n", " fract_z=0.5,\n", - " wyckoff_letter='c',\n", " adp_iso=1.66,\n", ")" ] @@ -2517,10 +2512,9 @@ "project_2.structures['si'].atom_sites.create(\n", " label='Si',\n", " type_symbol='Si',\n", - " fract_x=0,\n", - " fract_y=0,\n", - " fract_z=0,\n", - " wyckoff_letter='a',\n", + " fract_x=0.125,\n", + " fract_y=0.125,\n", + " fract_z=0.125,\n", " adp_iso=0.89,\n", ")\n", "\n", diff --git a/docs/docs/tutorials/ed-13.py b/docs/docs/tutorials/ed-13.py index 454f82502..82970780f 100644 --- a/docs/docs/tutorials/ed-13.py +++ b/docs/docs/tutorials/ed-13.py @@ -438,7 +438,7 @@ # _atom_site.occupancy # _atom_site.ADP_type # _atom_site.B_iso_or_equiv -# Si Si 0 0 0 a 1.0 Biso 0.89 +# Si Si 0.125 0.125 0.125 a 1.0 Biso 0.89 # ``` # %% [markdown] @@ -493,10 +493,9 @@ project_1.structures['si'].atom_sites.create( label='Si', type_symbol='Si', - fract_x=0, - fract_y=0, - fract_z=0, - wyckoff_letter='a', + fract_x=0.125, + fract_y=0.125, + fract_z=0.125, adp_iso=0.89, ) @@ -1030,7 +1029,6 @@ fract_x=0, fract_y=0, fract_z=0, - wyckoff_letter='a', adp_iso=0.95, occupancy=0.5, ) @@ -1040,7 +1038,6 @@ fract_x=0, fract_y=0, fract_z=0, - wyckoff_letter='a', adp_iso=0.95, occupancy=0.5, ) @@ -1050,7 +1047,6 @@ fract_x=0.5, fract_y=0.5, fract_z=0.5, - wyckoff_letter='b', adp_iso=0.80, ) project_2.structures['lbco'].atom_sites.create( @@ -1059,7 +1055,6 @@ fract_x=0, fract_y=0.5, fract_z=0.5, - wyckoff_letter='c', adp_iso=1.66, ) @@ -1408,10 +1403,9 @@ project_2.structures['si'].atom_sites.create( label='Si', type_symbol='Si', - fract_x=0, - fract_y=0, - fract_z=0, - wyckoff_letter='a', + fract_x=0.125, + fract_y=0.125, + fract_z=0.125, adp_iso=0.89, ) diff --git a/docs/docs/tutorials/ed-16.ipynb b/docs/docs/tutorials/ed-16.ipynb index 92092f299..09c816f05 100644 --- a/docs/docs/tutorials/ed-16.ipynb +++ b/docs/docs/tutorials/ed-16.ipynb @@ -137,7 +137,6 @@ " fract_x=0,\n", " fract_y=0,\n", " fract_z=0,\n", - " wyckoff_letter='a',\n", " adp_iso=0.2,\n", ")" ] diff --git a/docs/docs/tutorials/ed-16.py b/docs/docs/tutorials/ed-16.py index 2ede48cfb..89e60625f 100644 --- a/docs/docs/tutorials/ed-16.py +++ b/docs/docs/tutorials/ed-16.py @@ -52,7 +52,6 @@ fract_x=0, fract_y=0, fract_z=0, - wyckoff_letter='a', adp_iso=0.2, ) diff --git a/docs/docs/tutorials/ed-17.ipynb b/docs/docs/tutorials/ed-17.ipynb index 0c69f1348..351290746 100644 --- a/docs/docs/tutorials/ed-17.ipynb +++ b/docs/docs/tutorials/ed-17.ipynb @@ -177,7 +177,6 @@ " fract_x=0,\n", " fract_y=0,\n", " fract_z=0,\n", - " wyckoff_letter='a',\n", " adp_iso=0.3,\n", ")\n", "struct.atom_sites.create(\n", @@ -186,7 +185,6 @@ " fract_x=0.279,\n", " fract_y=0.25,\n", " fract_z=0.985,\n", - " wyckoff_letter='c',\n", " adp_iso=0.3,\n", ")\n", "struct.atom_sites.create(\n", @@ -195,7 +193,6 @@ " fract_x=0.094,\n", " fract_y=0.25,\n", " fract_z=0.429,\n", - " wyckoff_letter='c',\n", " adp_iso=0.34,\n", ")\n", "struct.atom_sites.create(\n", @@ -204,7 +201,6 @@ " fract_x=0.091,\n", " fract_y=0.25,\n", " fract_z=0.771,\n", - " wyckoff_letter='c',\n", " adp_iso=0.63,\n", ")\n", "struct.atom_sites.create(\n", @@ -213,7 +209,6 @@ " fract_x=0.448,\n", " fract_y=0.25,\n", " fract_z=0.217,\n", - " wyckoff_letter='c',\n", " adp_iso=0.59,\n", ")\n", "struct.atom_sites.create(\n", @@ -222,7 +217,6 @@ " fract_x=0.164,\n", " fract_y=0.032,\n", " fract_z=0.28,\n", - " wyckoff_letter='d',\n", " adp_iso=0.83,\n", ")" ] diff --git a/docs/docs/tutorials/ed-17.py b/docs/docs/tutorials/ed-17.py index e1879bd20..eb3605e9d 100644 --- a/docs/docs/tutorials/ed-17.py +++ b/docs/docs/tutorials/ed-17.py @@ -68,7 +68,6 @@ fract_x=0, fract_y=0, fract_z=0, - wyckoff_letter='a', adp_iso=0.3, ) struct.atom_sites.create( @@ -77,7 +76,6 @@ fract_x=0.279, fract_y=0.25, fract_z=0.985, - wyckoff_letter='c', adp_iso=0.3, ) struct.atom_sites.create( @@ -86,7 +84,6 @@ fract_x=0.094, fract_y=0.25, fract_z=0.429, - wyckoff_letter='c', adp_iso=0.34, ) struct.atom_sites.create( @@ -95,7 +92,6 @@ fract_x=0.091, fract_y=0.25, fract_z=0.771, - wyckoff_letter='c', adp_iso=0.63, ) struct.atom_sites.create( @@ -104,7 +100,6 @@ fract_x=0.448, fract_y=0.25, fract_z=0.217, - wyckoff_letter='c', adp_iso=0.59, ) struct.atom_sites.create( @@ -113,7 +108,6 @@ fract_x=0.164, fract_y=0.032, fract_z=0.28, - wyckoff_letter='d', adp_iso=0.83, ) diff --git a/docs/docs/tutorials/ed-2.ipynb b/docs/docs/tutorials/ed-2.ipynb index b2ca221f2..9b1872b5b 100644 --- a/docs/docs/tutorials/ed-2.ipynb +++ b/docs/docs/tutorials/ed-2.ipynb @@ -145,7 +145,6 @@ " fract_x=0,\n", " fract_y=0,\n", " fract_z=0,\n", - " wyckoff_letter='a',\n", " adp_iso=0.5,\n", " occupancy=0.5,\n", ")\n", @@ -155,7 +154,6 @@ " fract_x=0,\n", " fract_y=0,\n", " fract_z=0,\n", - " wyckoff_letter='a',\n", " adp_iso=0.5,\n", " occupancy=0.5,\n", ")\n", @@ -165,7 +163,6 @@ " fract_x=0.5,\n", " fract_y=0.5,\n", " fract_z=0.5,\n", - " wyckoff_letter='b',\n", " adp_iso=0.5,\n", ")\n", "structure.atom_sites.create(\n", @@ -174,7 +171,6 @@ " fract_x=0,\n", " fract_y=0.5,\n", " fract_z=0.5,\n", - " wyckoff_letter='c',\n", " adp_iso=0.5,\n", ")" ] diff --git a/docs/docs/tutorials/ed-2.py b/docs/docs/tutorials/ed-2.py index 5ede4537b..008ca5d0f 100644 --- a/docs/docs/tutorials/ed-2.py +++ b/docs/docs/tutorials/ed-2.py @@ -56,7 +56,6 @@ fract_x=0, fract_y=0, fract_z=0, - wyckoff_letter='a', adp_iso=0.5, occupancy=0.5, ) @@ -66,7 +65,6 @@ fract_x=0, fract_y=0, fract_z=0, - wyckoff_letter='a', adp_iso=0.5, occupancy=0.5, ) @@ -76,7 +74,6 @@ fract_x=0.5, fract_y=0.5, fract_z=0.5, - wyckoff_letter='b', adp_iso=0.5, ) structure.atom_sites.create( @@ -85,7 +82,6 @@ fract_x=0, fract_y=0.5, fract_z=0.5, - wyckoff_letter='c', adp_iso=0.5, ) diff --git a/docs/docs/tutorials/ed-20.ipynb b/docs/docs/tutorials/ed-20.ipynb index 4c32724d0..6dc22803e 100644 --- a/docs/docs/tutorials/ed-20.ipynb +++ b/docs/docs/tutorials/ed-20.ipynb @@ -90,7 +90,6 @@ " fract_x=0.0,\n", " fract_y=0.0,\n", " fract_z=0.0,\n", - " wyckoff_letter='a',\n", " adp_type='Biso',\n", " adp_iso=1.0,\n", ")" @@ -124,7 +123,6 @@ " fract_x=0.0,\n", " fract_y=0.0,\n", " fract_z=0.0,\n", - " wyckoff_letter='a',\n", " adp_type='Biso',\n", " adp_iso=1.0,\n", ")" diff --git a/docs/docs/tutorials/ed-20.py b/docs/docs/tutorials/ed-20.py index b54a090b3..5b4b6fe03 100644 --- a/docs/docs/tutorials/ed-20.py +++ b/docs/docs/tutorials/ed-20.py @@ -41,7 +41,6 @@ fract_x=0.0, fract_y=0.0, fract_z=0.0, - wyckoff_letter='a', adp_type='Biso', adp_iso=1.0, ) @@ -63,7 +62,6 @@ fract_x=0.0, fract_y=0.0, fract_z=0.0, - wyckoff_letter='a', adp_type='Biso', adp_iso=1.0, ) diff --git a/docs/docs/tutorials/ed-3.ipynb b/docs/docs/tutorials/ed-3.ipynb index 8076de33a..cce439297 100644 --- a/docs/docs/tutorials/ed-3.ipynb +++ b/docs/docs/tutorials/ed-3.ipynb @@ -267,7 +267,6 @@ " fract_x=0,\n", " fract_y=0,\n", " fract_z=0,\n", - " wyckoff_letter='a',\n", " adp_iso=0.5,\n", " occupancy=0.5,\n", ")\n", @@ -277,7 +276,6 @@ " fract_x=0,\n", " fract_y=0,\n", " fract_z=0,\n", - " wyckoff_letter='a',\n", " adp_iso=0.5,\n", " occupancy=0.5,\n", ")\n", @@ -287,7 +285,6 @@ " fract_x=0.5,\n", " fract_y=0.5,\n", " fract_z=0.5,\n", - " wyckoff_letter='b',\n", " adp_iso=0.5,\n", ")\n", "project.structures['lbco'].atom_sites.create(\n", @@ -296,7 +293,6 @@ " fract_x=0,\n", " fract_y=0.5,\n", " fract_z=0.5,\n", - " wyckoff_letter='c',\n", " adp_iso=0.5,\n", ")" ] diff --git a/docs/docs/tutorials/ed-3.py b/docs/docs/tutorials/ed-3.py index dff71180b..6b1c06dc1 100644 --- a/docs/docs/tutorials/ed-3.py +++ b/docs/docs/tutorials/ed-3.py @@ -112,7 +112,6 @@ fract_x=0, fract_y=0, fract_z=0, - wyckoff_letter='a', adp_iso=0.5, occupancy=0.5, ) @@ -122,7 +121,6 @@ fract_x=0, fract_y=0, fract_z=0, - wyckoff_letter='a', adp_iso=0.5, occupancy=0.5, ) @@ -132,7 +130,6 @@ fract_x=0.5, fract_y=0.5, fract_z=0.5, - wyckoff_letter='b', adp_iso=0.5, ) project.structures['lbco'].atom_sites.create( @@ -141,7 +138,6 @@ fract_x=0, fract_y=0.5, fract_z=0.5, - wyckoff_letter='c', adp_iso=0.5, ) diff --git a/docs/docs/tutorials/ed-4.ipynb b/docs/docs/tutorials/ed-4.ipynb index 2bc849990..c45ed29cf 100644 --- a/docs/docs/tutorials/ed-4.ipynb +++ b/docs/docs/tutorials/ed-4.ipynb @@ -142,7 +142,6 @@ " fract_x=0.1876,\n", " fract_y=0.25,\n", " fract_z=0.167,\n", - " wyckoff_letter='c',\n", " adp_iso=1.37,\n", ")\n", "structure.atom_sites.create(\n", @@ -151,7 +150,6 @@ " fract_x=0.0654,\n", " fract_y=0.25,\n", " fract_z=0.684,\n", - " wyckoff_letter='c',\n", " adp_iso=0.3777,\n", ")\n", "structure.atom_sites.create(\n", @@ -160,7 +158,6 @@ " fract_x=0.9082,\n", " fract_y=0.25,\n", " fract_z=0.5954,\n", - " wyckoff_letter='c',\n", " adp_iso=1.9764,\n", ")\n", "structure.atom_sites.create(\n", @@ -169,7 +166,6 @@ " fract_x=0.1935,\n", " fract_y=0.25,\n", " fract_z=0.5432,\n", - " wyckoff_letter='c',\n", " adp_iso=1.4456,\n", ")\n", "structure.atom_sites.create(\n", @@ -178,7 +174,6 @@ " fract_x=0.0811,\n", " fract_y=0.0272,\n", " fract_z=0.8086,\n", - " wyckoff_letter='d',\n", " adp_iso=1.2822,\n", ")" ] diff --git a/docs/docs/tutorials/ed-4.py b/docs/docs/tutorials/ed-4.py index 39ea4b150..0e974893a 100644 --- a/docs/docs/tutorials/ed-4.py +++ b/docs/docs/tutorials/ed-4.py @@ -55,7 +55,6 @@ fract_x=0.1876, fract_y=0.25, fract_z=0.167, - wyckoff_letter='c', adp_iso=1.37, ) structure.atom_sites.create( @@ -64,7 +63,6 @@ fract_x=0.0654, fract_y=0.25, fract_z=0.684, - wyckoff_letter='c', adp_iso=0.3777, ) structure.atom_sites.create( @@ -73,7 +71,6 @@ fract_x=0.9082, fract_y=0.25, fract_z=0.5954, - wyckoff_letter='c', adp_iso=1.9764, ) structure.atom_sites.create( @@ -82,7 +79,6 @@ fract_x=0.1935, fract_y=0.25, fract_z=0.5432, - wyckoff_letter='c', adp_iso=1.4456, ) structure.atom_sites.create( @@ -91,7 +87,6 @@ fract_x=0.0811, fract_y=0.0272, fract_z=0.8086, - wyckoff_letter='d', adp_iso=1.2822, ) diff --git a/docs/docs/tutorials/ed-5.ipynb b/docs/docs/tutorials/ed-5.ipynb index 6aa0fedb2..391ca3651 100644 --- a/docs/docs/tutorials/ed-5.ipynb +++ b/docs/docs/tutorials/ed-5.ipynb @@ -138,7 +138,6 @@ " fract_x=0,\n", " fract_y=0,\n", " fract_z=0,\n", - " wyckoff_letter='a',\n", " adp_iso=0.5,\n", ")\n", "structure.atom_sites.create(\n", @@ -147,7 +146,6 @@ " fract_x=0.279,\n", " fract_y=0.25,\n", " fract_z=0.985,\n", - " wyckoff_letter='c',\n", " adp_iso=0.5,\n", ")\n", "structure.atom_sites.create(\n", @@ -156,7 +154,6 @@ " fract_x=0.094,\n", " fract_y=0.25,\n", " fract_z=0.429,\n", - " wyckoff_letter='c',\n", " adp_iso=0.5,\n", ")\n", "structure.atom_sites.create(\n", @@ -165,7 +162,6 @@ " fract_x=0.091,\n", " fract_y=0.25,\n", " fract_z=0.771,\n", - " wyckoff_letter='c',\n", " adp_iso=0.5,\n", ")\n", "structure.atom_sites.create(\n", @@ -174,7 +170,6 @@ " fract_x=0.448,\n", " fract_y=0.25,\n", " fract_z=0.217,\n", - " wyckoff_letter='c',\n", " adp_iso=0.5,\n", ")\n", "structure.atom_sites.create(\n", @@ -183,7 +178,6 @@ " fract_x=0.164,\n", " fract_y=0.032,\n", " fract_z=0.28,\n", - " wyckoff_letter='d',\n", " adp_iso=0.5,\n", ")" ] diff --git a/docs/docs/tutorials/ed-5.py b/docs/docs/tutorials/ed-5.py index 1e1c27dda..942444d25 100644 --- a/docs/docs/tutorials/ed-5.py +++ b/docs/docs/tutorials/ed-5.py @@ -53,7 +53,6 @@ fract_x=0, fract_y=0, fract_z=0, - wyckoff_letter='a', adp_iso=0.5, ) structure.atom_sites.create( @@ -62,7 +61,6 @@ fract_x=0.279, fract_y=0.25, fract_z=0.985, - wyckoff_letter='c', adp_iso=0.5, ) structure.atom_sites.create( @@ -71,7 +69,6 @@ fract_x=0.094, fract_y=0.25, fract_z=0.429, - wyckoff_letter='c', adp_iso=0.5, ) structure.atom_sites.create( @@ -80,7 +77,6 @@ fract_x=0.091, fract_y=0.25, fract_z=0.771, - wyckoff_letter='c', adp_iso=0.5, ) structure.atom_sites.create( @@ -89,7 +85,6 @@ fract_x=0.448, fract_y=0.25, fract_z=0.217, - wyckoff_letter='c', adp_iso=0.5, ) structure.atom_sites.create( @@ -98,7 +93,6 @@ fract_x=0.164, fract_y=0.032, fract_z=0.28, - wyckoff_letter='d', adp_iso=0.5, ) diff --git a/docs/docs/tutorials/ed-6.ipynb b/docs/docs/tutorials/ed-6.ipynb index 488690968..561afb750 100644 --- a/docs/docs/tutorials/ed-6.ipynb +++ b/docs/docs/tutorials/ed-6.ipynb @@ -136,7 +136,6 @@ " fract_x=0,\n", " fract_y=0,\n", " fract_z=0.5,\n", - " wyckoff_letter='b',\n", " adp_iso=0.5,\n", ")\n", "structure.atom_sites.create(\n", @@ -145,7 +144,6 @@ " fract_x=0.5,\n", " fract_y=0,\n", " fract_z=0,\n", - " wyckoff_letter='e',\n", " adp_iso=0.5,\n", ")\n", "structure.atom_sites.create(\n", @@ -154,7 +152,6 @@ " fract_x=0.21,\n", " fract_y=-0.21,\n", " fract_z=0.06,\n", - " wyckoff_letter='h',\n", " adp_iso=0.5,\n", ")\n", "structure.atom_sites.create(\n", @@ -163,7 +160,6 @@ " fract_x=0,\n", " fract_y=0,\n", " fract_z=0.197,\n", - " wyckoff_letter='c',\n", " adp_iso=0.5,\n", ")\n", "structure.atom_sites.create(\n", @@ -172,7 +168,6 @@ " fract_x=0.13,\n", " fract_y=-0.13,\n", " fract_z=0.08,\n", - " wyckoff_letter='h',\n", " adp_iso=0.5,\n", ")" ] diff --git a/docs/docs/tutorials/ed-6.py b/docs/docs/tutorials/ed-6.py index b57c56952..f2968b774 100644 --- a/docs/docs/tutorials/ed-6.py +++ b/docs/docs/tutorials/ed-6.py @@ -50,7 +50,6 @@ fract_x=0, fract_y=0, fract_z=0.5, - wyckoff_letter='b', adp_iso=0.5, ) structure.atom_sites.create( @@ -59,7 +58,6 @@ fract_x=0.5, fract_y=0, fract_z=0, - wyckoff_letter='e', adp_iso=0.5, ) structure.atom_sites.create( @@ -68,7 +66,6 @@ fract_x=0.21, fract_y=-0.21, fract_z=0.06, - wyckoff_letter='h', adp_iso=0.5, ) structure.atom_sites.create( @@ -77,7 +74,6 @@ fract_x=0, fract_y=0, fract_z=0.197, - wyckoff_letter='c', adp_iso=0.5, ) structure.atom_sites.create( @@ -86,7 +82,6 @@ fract_x=0.13, fract_y=-0.13, fract_z=0.08, - wyckoff_letter='h', adp_iso=0.5, ) diff --git a/docs/docs/tutorials/ed-8.ipynb b/docs/docs/tutorials/ed-8.ipynb index e564ca387..7e02e5770 100644 --- a/docs/docs/tutorials/ed-8.ipynb +++ b/docs/docs/tutorials/ed-8.ipynb @@ -136,7 +136,6 @@ " fract_x=0.4663,\n", " fract_y=0.0,\n", " fract_z=0.25,\n", - " wyckoff_letter='b',\n", " adp_iso=0.92,\n", ")\n", "structure.atom_sites.create(\n", @@ -145,7 +144,6 @@ " fract_x=0.2521,\n", " fract_y=0.2521,\n", " fract_z=0.2521,\n", - " wyckoff_letter='a',\n", " adp_iso=0.73,\n", ")\n", "structure.atom_sites.create(\n", @@ -154,7 +152,6 @@ " fract_x=0.0851,\n", " fract_y=0.0851,\n", " fract_z=0.0851,\n", - " wyckoff_letter='a',\n", " adp_iso=2.08,\n", ")\n", "structure.atom_sites.create(\n", @@ -163,7 +160,6 @@ " fract_x=0.1377,\n", " fract_y=0.3054,\n", " fract_z=0.1195,\n", - " wyckoff_letter='c',\n", " adp_iso=0.90,\n", ")\n", "structure.atom_sites.create(\n", @@ -172,7 +168,6 @@ " fract_x=0.3625,\n", " fract_y=0.3633,\n", " fract_z=0.1867,\n", - " wyckoff_letter='c',\n", " adp_iso=1.37,\n", ")\n", "structure.atom_sites.create(\n", @@ -181,7 +176,6 @@ " fract_x=0.4612,\n", " fract_y=0.4612,\n", " fract_z=0.4612,\n", - " wyckoff_letter='a',\n", " adp_iso=0.88,\n", ")" ] diff --git a/docs/docs/tutorials/ed-8.py b/docs/docs/tutorials/ed-8.py index 09917e35c..d875c3998 100644 --- a/docs/docs/tutorials/ed-8.py +++ b/docs/docs/tutorials/ed-8.py @@ -51,7 +51,6 @@ fract_x=0.4663, fract_y=0.0, fract_z=0.25, - wyckoff_letter='b', adp_iso=0.92, ) structure.atom_sites.create( @@ -60,7 +59,6 @@ fract_x=0.2521, fract_y=0.2521, fract_z=0.2521, - wyckoff_letter='a', adp_iso=0.73, ) structure.atom_sites.create( @@ -69,7 +67,6 @@ fract_x=0.0851, fract_y=0.0851, fract_z=0.0851, - wyckoff_letter='a', adp_iso=2.08, ) structure.atom_sites.create( @@ -78,7 +75,6 @@ fract_x=0.1377, fract_y=0.3054, fract_z=0.1195, - wyckoff_letter='c', adp_iso=0.90, ) structure.atom_sites.create( @@ -87,7 +83,6 @@ fract_x=0.3625, fract_y=0.3633, fract_z=0.1867, - wyckoff_letter='c', adp_iso=1.37, ) structure.atom_sites.create( @@ -96,7 +91,6 @@ fract_x=0.4612, fract_y=0.4612, fract_z=0.4612, - wyckoff_letter='a', adp_iso=0.88, ) diff --git a/docs/docs/tutorials/ed-9.ipynb b/docs/docs/tutorials/ed-9.ipynb index 8722b614e..8d1ecdfee 100644 --- a/docs/docs/tutorials/ed-9.ipynb +++ b/docs/docs/tutorials/ed-9.ipynb @@ -133,7 +133,6 @@ " fract_x=0,\n", " fract_y=0,\n", " fract_z=0,\n", - " wyckoff_letter='a',\n", " adp_iso=0.2,\n", " occupancy=0.5,\n", ")\n", @@ -143,7 +142,6 @@ " fract_x=0,\n", " fract_y=0,\n", " fract_z=0,\n", - " wyckoff_letter='a',\n", " adp_iso=0.2,\n", " occupancy=0.5,\n", ")\n", @@ -153,7 +151,6 @@ " fract_x=0.5,\n", " fract_y=0.5,\n", " fract_z=0.5,\n", - " wyckoff_letter='b',\n", " adp_iso=0.2567,\n", ")\n", "structure_1.atom_sites.create(\n", @@ -162,7 +159,6 @@ " fract_x=0,\n", " fract_y=0.5,\n", " fract_z=0.5,\n", - " wyckoff_letter='c',\n", " adp_iso=1.4041,\n", ")" ] @@ -243,7 +239,6 @@ " fract_x=0.0,\n", " fract_y=0.0,\n", " fract_z=0.0,\n", - " wyckoff_letter='a',\n", " adp_iso=0.0,\n", ")" ] diff --git a/docs/docs/tutorials/ed-9.py b/docs/docs/tutorials/ed-9.py index 083bde68f..4fe451af2 100644 --- a/docs/docs/tutorials/ed-9.py +++ b/docs/docs/tutorials/ed-9.py @@ -48,7 +48,6 @@ fract_x=0, fract_y=0, fract_z=0, - wyckoff_letter='a', adp_iso=0.2, occupancy=0.5, ) @@ -58,7 +57,6 @@ fract_x=0, fract_y=0, fract_z=0, - wyckoff_letter='a', adp_iso=0.2, occupancy=0.5, ) @@ -68,7 +66,6 @@ fract_x=0.5, fract_y=0.5, fract_z=0.5, - wyckoff_letter='b', adp_iso=0.2567, ) structure_1.atom_sites.create( @@ -77,7 +74,6 @@ fract_x=0, fract_y=0.5, fract_z=0.5, - wyckoff_letter='c', adp_iso=1.4041, ) @@ -110,7 +106,6 @@ fract_x=0.0, fract_y=0.0, fract_z=0.0, - wyckoff_letter='a', adp_iso=0.0, ) diff --git a/src/easydiffraction/analysis/calculators/cryspy.py b/src/easydiffraction/analysis/calculators/cryspy.py index c39db75d5..16fcd818b 100644 --- a/src/easydiffraction/analysis/calculators/cryspy.py +++ b/src/easydiffraction/analysis/calculators/cryspy.py @@ -489,36 +489,25 @@ def _update_atom_multiplicity( structure: Structure, ) -> None: """ - Update cryspy atom multiplicities. + Update cryspy atom multiplicities from the model. CrysPy normalizes fractional coordinates into the ``[0, 1)`` interval while parsing CIF. For sites such as ``(x, -x, z)``, that can turn ``-x`` into ``1 - x`` before the Wyckoff multiplicity is inferred, making special positions look like - general positions. EasyDiffraction already stores the intended - Wyckoff letter, so keep the calculator dictionary aligned with - that model state. + general positions. EasyDiffraction's Wyckoff detection already + stores the correct per-site multiplicity, so use it; when a site + has no detected multiplicity (untabulated space group), keep the + backend's inferred value. """ if cryspy is None: return - from cryspy.A_functions_base.function_2_space_group import ( # noqa: PLC0415 - get_it_number_by_name_hm_short, - ) - - from easydiffraction.crystallography.space_groups import SPACE_GROUPS # noqa: PLC0415 - - it_number = get_it_number_by_name_hm_short(structure.space_group.name_h_m.value) - coord_code = structure.space_group.it_coordinate_system_code.value - if it_number is None or (it_number, coord_code) not in SPACE_GROUPS: - return - - positions = SPACE_GROUPS[it_number, coord_code]['Wyckoff_positions'] multiplicity = cryspy_model_dict['atom_multiplicity'] for idx, atom_site in enumerate(structure.atom_sites): - wyckoff_letter = atom_site.wyckoff_letter.value - if wyckoff_letter in positions: - multiplicity[idx] = positions[wyckoff_letter]['multiplicity'] + site_multiplicity = atom_site.multiplicity.value + if site_multiplicity is not None: + multiplicity[idx] = site_multiplicity @staticmethod def _update_aniso_beta( diff --git a/src/easydiffraction/analysis/fitting.py b/src/easydiffraction/analysis/fitting.py index 7dd8c731d..5b55f34df 100644 --- a/src/easydiffraction/analysis/fitting.py +++ b/src/easydiffraction/analysis/fitting.py @@ -377,9 +377,12 @@ def _residual_function( # Update categories to reflect new parameter values # Order matters: structures first (symmetry, structure), - # then analysis (constraints), then experiments (calculations) + # then analysis (constraints), then experiments (calculations). + # Pass called_by_minimizer so the per-iteration Wyckoff snap + # runs silently (no re-detection or warnings); detection + # already ran at fit setup. for structure in structures: - structure._update_categories() + structure._update_categories(called_by_minimizer=True) if analysis is not None: analysis._update_categories(called_by_minimizer=True) diff --git a/src/easydiffraction/core/validation.py b/src/easydiffraction/core/validation.py index 546719fe9..680edd82e 100644 --- a/src/easydiffraction/core/validation.py +++ b/src/easydiffraction/core/validation.py @@ -252,6 +252,36 @@ def validated( # ====================================================================== +class PermissiveMembershipValidator(MembershipValidator): + """ + Membership validator accepting any value when choices are empty. + + Used where the allowed set is derived dynamically and may + legitimately be empty (for example a Wyckoff letter under an + untabulated space group, or before a parent context is available): + an empty allowed set stores the value verbatim instead of rejecting + it. A non-empty allowed set validates membership as usual. + """ + + def validated( + self, + value: object, + name: str, + default: object = None, + current: object = None, + ) -> object: + """ + Accept any value when allowed is empty, else check membership. + """ + allowed_values = self.allowed() if callable(self.allowed) else self.allowed + if not allowed_values: + return value + return super().validated(value, name, default=default, current=current) + + +# ====================================================================== + + class RegexValidator(ValidatorBase): """Ensure that a string matches a given regular expression.""" diff --git a/src/easydiffraction/crystallography/__init__.py b/src/easydiffraction/crystallography/__init__.py index 4e798e209..080e2094d 100644 --- a/src/easydiffraction/crystallography/__init__.py +++ b/src/easydiffraction/crystallography/__init__.py @@ -1,2 +1,6 @@ # SPDX-FileCopyrightText: 2026 EasyScience contributors # SPDX-License-Identifier: BSD-3-Clause + +from easydiffraction.crystallography.crystallography import WyckoffPosition +from easydiffraction.crystallography.crystallography import detect_wyckoff_position +from easydiffraction.crystallography.crystallography import wyckoff_position_info diff --git a/src/easydiffraction/crystallography/crystallography.py b/src/easydiffraction/crystallography/crystallography.py index 8359d551a..e75569a4b 100644 --- a/src/easydiffraction/crystallography/crystallography.py +++ b/src/easydiffraction/crystallography/crystallography.py @@ -1,6 +1,9 @@ # SPDX-FileCopyrightText: 2025 EasyScience contributors # SPDX-License-Identifier: BSD-3-Clause +import itertools +import operator +from dataclasses import dataclass from fractions import Fraction from typing import Any @@ -16,6 +19,59 @@ from easydiffraction.crystallography.space_groups import SPACE_GROUPS from easydiffraction.utils.logging import log +# Maximum residual (in fractional units) for a coordinate to be +# considered on a Wyckoff orbit during detection. +_WYCKOFF_DETECTION_TOL = 1e-3 + + +@dataclass(frozen=True) +class WyckoffPosition: + """ + A resolved Wyckoff position and the orbit representative matched. + + Attributes + ---------- + letter : str + Wyckoff letter (e.g. ``'h'``). + multiplicity : int + Site multiplicity (the full orbit size). + site_symmetry : str + International Tables site-symmetry symbol. + coord_template : str | None + Nearest matched orbit representative (e.g. ``'(x,-x,z)'``), or + ``None`` for a table lookup made without coordinates. This is + what coordinate snapping and constrained-axis flags consume. + """ + + letter: str + multiplicity: int + site_symmetry: str + coord_template: str | None + + +def _normalize_coord_code(coord_code: str | None) -> str | None: + """ + Normalize a coordinate-system code to the ``SPACE_GROUPS`` key form. + + The empty string (used by consumers for groups with no coordinate + code, e.g. triclinic P1/P-1) maps to ``None``, which is how those + settings are keyed in ``SPACE_GROUPS``. + + Parameters + ---------- + coord_code : str | None + Incoming coordinate-system code. + + Returns + ------- + str | None + ``None`` for the empty string, otherwise ``coord_code`` + unchanged. + """ + if not coord_code: + return None + return coord_code + def apply_cell_symmetry_constraints( cell: dict[str, float], @@ -202,14 +258,13 @@ def _get_wyckoff_exprs( log.error(f"Failed to get IT_number for name_H-M '{name_hm}'") return None - if coord_code is None: - log.error('IT_coordinate_system_code is not set') - return None - + coord_code = _normalize_coord_code(coord_code) if (it_number, coord_code) not in SPACE_GROUPS: - # Space group is not in the local SPACE_GROUPS table (e.g. P 1, - # where cryspy reports no coordinate-system codes). Treat as - # "no symmetry constraints to apply". + # Space group / coordinate-system combination is absent + # from the local SPACE_GROUPS table. Treat as "no symmetry + # constraints to apply". Triclinic groups are keyed + # ``(it_number, None)`` and resolve normally through this + # lookup, so a ``None`` code is not treated as unset. return None entry = SPACE_GROUPS[it_number, coord_code] @@ -415,7 +470,7 @@ def _get_general_position_ops( list[tuple[np.ndarray, np.ndarray]] | None List of (rotation, translation) pairs, or ``None`` on failure. """ - key = (it_number, coord_code) + key = (it_number, _normalize_coord_code(coord_code)) if key not in SPACE_GROUPS: # Not in the local SPACE_GROUPS table (e.g. P 1, where cryspy # reports no coordinate-system codes). The caller falls back to @@ -430,6 +485,287 @@ def _get_general_position_ops( return [_parse_rotation_matrix(c) for c in general_coords] +def _orbit_template_residual(point: np.ndarray, rot: np.ndarray, trans: np.ndarray) -> float: + """ + Return the mod-1 distance from ``point`` to an orbit template. + + The template manifold is ``{rot·v + trans}``; the point lies on it + for some free ``v`` when the residual is ~0. Integer lattice shifts + account for unit-cell periodicity. + + Parameters + ---------- + point : np.ndarray + Fractional coordinate reduced into the unit cell. + rot : np.ndarray + (3, 3) rotation part of the template. + trans : np.ndarray + (3,) translation part of the template. + + Returns + ------- + float + Smallest residual over the candidate lattice shifts. + """ + rot_float = rot.astype(float) + base = point - trans + best = np.inf + for shift in itertools.product((-1.0, 0.0, 1.0), repeat=3): + rhs = base - np.array(shift) + solution, *_ = np.linalg.lstsq(rot_float, rhs, rcond=None) + residual = float(np.linalg.norm(rot_float @ solution - rhs)) + best = min(best, residual) + return best + + +def _nearest_orbit_template(point: np.ndarray, coords_xyz: list[str]) -> tuple[str, float]: + """ + Return the orbit template nearest to ``point`` and its residual. + + On near-ties (e.g. centering copies that share a manifold mod 1) the + earlier, canonical representative is kept for deterministic output; + the free-parameter-solving snap downstream is correct for any of + them. + """ + best_template = coords_xyz[0] + best_residual = np.inf + for template in coords_xyz: + rot, trans = _parse_rotation_matrix(template) + residual = _orbit_template_residual(point, rot, trans) + if residual < best_residual - 1e-9: + best_residual = residual + best_template = template + return best_template, best_residual + + +def _wyckoff_template_constrained_flags(rot: np.ndarray) -> dict[str, bool]: + """ + Return per-axis symmetry-constraint flags for a parsed template. + + An axis is **free** when its coordinate is the first (in x, y, z + order) to introduce a free parameter, and **constrained** otherwise + (a constant, or a coordinate slaved to an earlier axis's parameter). + This is slot-based, so it is correct for off-canonical + representatives such as ``(0,x,0)`` (``fract_x`` constrained, + ``fract_y`` free) where the symbol-presence test would be wrong. + + Parameters + ---------- + rot : np.ndarray + (3, 3) rotation part from :func:`_parse_rotation_matrix`. + + Returns + ------- + dict[str, bool] + Mapping ``'fract_x'/'fract_y'/'fract_z'`` to ``True`` if the + axis is fully fixed by site symmetry. + """ + claimed: set[int] = set() + flags: dict[str, bool] = {} + for axis, name in enumerate(('fract_x', 'fract_y', 'fract_z')): + used = {col for col in range(3) if rot[axis, col] != 0} + if used - claimed: + flags[name] = False + claimed |= used + else: + flags[name] = True + return flags + + +def snap_to_wyckoff_template( + coord_template: str, + fract_xyz: tuple[float, float, float], +) -> tuple[tuple[float, float, float], dict[str, bool]]: + """ + Project a coordinate onto a Wyckoff orbit-representative manifold. + + Solves the free Wyckoff parameters from the **free (refinable) + axes** only — keeping those axes' values, so a minimizer's refined + free coordinate is preserved — then derives the constrained axes + from the template. Replaces per-axis symbol substitution: it handles + coupled axes (e.g. ``(x,-x,z)``: keep ``fract_x``, set + ``fract_y=-fract_x``) and off-canonical representatives (e.g. + ``(0,x,0)``: keep ``fract_y``, set ``fract_x=fract_z=0``). + + Parameters + ---------- + coord_template : str + Selected orbit representative, e.g. ``'(x,-x,z)'`` or + ``'(0,x,0)'``. + fract_xyz : tuple[float, float, float] + Current fractional coordinate. + + Returns + ------- + tuple[tuple[float, float, float], dict[str, bool]] + The snapped ``(x, y, z)`` (free axes kept, constrained axes + derived; not reduced mod 1), and the per-axis constraint flags. + """ + rot, trans = _parse_rotation_matrix(coord_template) + rot_float = rot.astype(float) + point = np.asarray(fract_xyz, dtype=float) + axes = ('fract_x', 'fract_y', 'fract_z') + flags = _wyckoff_template_constrained_flags(rot) + free_rows = [axis for axis, name in enumerate(axes) if not flags[name]] + if free_rows: + solution, *_ = np.linalg.lstsq( + rot_float[free_rows, :], + point[free_rows] - trans[free_rows], + rcond=None, + ) + else: + solution = np.zeros(3) + derived = rot_float @ solution + trans + snapped = tuple( + float(point[axis]) if not flags[name] else float(derived[axis]) + for axis, name in enumerate(axes) + ) + return snapped, flags + + +def detect_wyckoff_position( + name_hm: str, + coord_code: str | None, + fract_xyz: tuple[float, float, float], + tol: float = _WYCKOFF_DETECTION_TOL, +) -> WyckoffPosition | None: + """ + Detect the Wyckoff position a fractional coordinate occupies. + + Tests the coordinate for membership in every Wyckoff orbit of the + resolved space group and returns the matched position with its + nearest representative template. The winner is chosen by + multiplicity ascending, then residual ascending (the most special + site first); a rare same-multiplicity tie within ``tol`` is reported + with a warning. + + Parameters + ---------- + name_hm : str + Hermann-Mauguin symbol of the space group. + coord_code : str | None + Coordinate-system code. + fract_xyz : tuple[float, float, float] + Fractional coordinate to classify. + tol : float, default=_WYCKOFF_DETECTION_TOL + Maximum residual for orbit membership. + + Returns + ------- + WyckoffPosition | None + The matched position, or ``None`` when the space group is absent + from ``SPACE_GROUPS``. + """ + it_number = get_it_number_by_name_hm_short(name_hm) + if it_number is None: + return None + key = (it_number, _normalize_coord_code(coord_code)) + if key not in SPACE_GROUPS: + return None + + point = np.asarray(fract_xyz, dtype=float) % 1.0 + matches = [] + for letter, position in SPACE_GROUPS[key]['Wyckoff_positions'].items(): + template, residual = _nearest_orbit_template(point, position['coords_xyz']) + if residual <= tol: + matches.append( + ( + int(position['multiplicity']), + residual, + letter, + position['site_symmetry'], + template, + ), + ) + + if not matches: + return None + matches.sort(key=operator.itemgetter(0, 1)) + multiplicity, _residual, letter, site_symmetry, template = matches[0] + ties = [match for match in matches[1:] if match[0] == multiplicity] + if ties: + others = ', '.join(repr(match[2]) for match in ties) + log.warning( + f'Wyckoff detection tie for {name_hm!r}: chose {letter!r} ' + f'over {others} (same multiplicity)', + ) + return WyckoffPosition(letter, multiplicity, site_symmetry, template) + + +def wyckoff_position_info( + name_hm: str, + coord_code: str | None, + letter: str, + fract_xyz: tuple[float, float, float] | None = None, +) -> WyckoffPosition | None: + """ + Look up a Wyckoff letter and optionally its representative. + + Parameters + ---------- + name_hm : str + Hermann-Mauguin symbol of the space group. + coord_code : str | None + Coordinate-system code. + letter : str + Wyckoff letter to look up. + fract_xyz : tuple[float, float, float] | None, default=None + When given, the nearest orbit representative for ``letter`` is + selected as ``coord_template``; otherwise ``coord_template`` is + ``None``. + + Returns + ------- + WyckoffPosition | None + The position record, or ``None`` when the group or letter is + absent. + """ + it_number = get_it_number_by_name_hm_short(name_hm) + if it_number is None: + return None + key = (it_number, _normalize_coord_code(coord_code)) + if key not in SPACE_GROUPS: + return None + positions = SPACE_GROUPS[key]['Wyckoff_positions'] + if letter not in positions: + return None + position = positions[letter] + template = None + if fract_xyz is not None: + point = np.asarray(fract_xyz, dtype=float) % 1.0 + template, _residual = _nearest_orbit_template(point, position['coords_xyz']) + return WyckoffPosition( + letter, int(position['multiplicity']), position['site_symmetry'], template + ) + + +def space_group_wyckoff_table(name_hm: str, coord_code: str | None) -> dict[str, dict] | None: + """ + Return the Wyckoff-position table for a space group, or ``None``. + + Parameters + ---------- + name_hm : str + Hermann-Mauguin symbol of the space group. + coord_code : str | None + Coordinate-system code. + + Returns + ------- + dict[str, dict] | None + Mapping of Wyckoff letter to its ``multiplicity``, + ``site_symmetry``, and ``coords_xyz`` record, or ``None`` when + the space group is absent from ``SPACE_GROUPS``. + """ + it_number = get_it_number_by_name_hm_short(name_hm) + if it_number is None: + return None + key = (it_number, _normalize_coord_code(coord_code)) + if key not in SPACE_GROUPS: + return None + return SPACE_GROUPS[key]['Wyckoff_positions'] + + def _site_stabilizer_rotations( ops: list[tuple[np.ndarray, np.ndarray]], site_coords: tuple[float, float, float], diff --git a/src/easydiffraction/crystallography/space_groups.json.gz b/src/easydiffraction/crystallography/space_groups.json.gz index 75d4f80ce..198668e13 100644 Binary files a/src/easydiffraction/crystallography/space_groups.json.gz and b/src/easydiffraction/crystallography/space_groups.json.gz differ diff --git a/src/easydiffraction/datablocks/structure/categories/atom_sites/default.py b/src/easydiffraction/datablocks/structure/categories/atom_sites/default.py index 6f7169c9f..7db19694c 100644 --- a/src/easydiffraction/datablocks/structure/categories/atom_sites/default.py +++ b/src/easydiffraction/datablocks/structure/categories/atom_sites/default.py @@ -19,15 +19,18 @@ from easydiffraction.core.metadata import TypeInfo from easydiffraction.core.validation import AttributeSpec from easydiffraction.core.validation import MembershipValidator +from easydiffraction.core.validation import PermissiveMembershipValidator from easydiffraction.core.validation import RangeValidator from easydiffraction.core.validation import RegexValidator from easydiffraction.core.variable import EnumDescriptor +from easydiffraction.core.variable import IntegerDescriptor from easydiffraction.core.variable import Parameter from easydiffraction.core.variable import StringDescriptor from easydiffraction.crystallography import crystallography as ecr from easydiffraction.datablocks.structure.categories.atom_sites.enums import AdpTypeEnum from easydiffraction.datablocks.structure.categories.atom_sites.factory import AtomSitesFactory from easydiffraction.io.cif.handler import CifHandler +from easydiffraction.utils.logging import log class AtomSite(CategoryItem): @@ -44,6 +47,14 @@ class AtomSite(CategoryItem): def __init__(self) -> None: """Initialise the atom site with default descriptor values.""" super().__init__() + # Set when a Wyckoff letter is assigned without a parent context + # (e.g. create() before the atom is added); the update flow then + # validates it once the parent structure is available. + self._wyckoff_letter_needs_validation = False + # Wyckoff-detection baselines (None until first detection); + # compared in the update flow to decide whether to re-detect. + self._wyckoff_coord_baseline: tuple[float, float, float] | None = None + self._wyckoff_key_baseline: tuple[str, str | None] | None = None self._label = StringDescriptor( name='label', @@ -123,7 +134,9 @@ def __init__(self) -> None: ), value_spec=AttributeSpec( default=self._wyckoff_letter_default_value, - validator=MembershipValidator(allowed=self._wyckoff_letter_allowed_values), + validator=PermissiveMembershipValidator( + allowed=lambda: self._wyckoff_letter_allowed_values, + ), ), cif_handler=CifHandler( names=[ @@ -133,6 +146,17 @@ def __init__(self) -> None: ] ), ) + self._multiplicity = IntegerDescriptor( + name='multiplicity', + description='Site multiplicity derived from the Wyckoff ' + 'position; None for an untabulated space group.', + display_handler=DisplayHandler( + display_name='Mult.', + latex_name='Mult.', + ), + value_spec=AttributeSpec(default=None, allow_none=True), + cif_handler=CifHandler(names=['_atom_site.site_symmetry_multiplicity']), + ) self._occupancy = Parameter( name='occupancy', description='Occupancy of the atom site, representing the ' @@ -196,21 +220,39 @@ def _type_symbol_allowed_values(self) -> list[str]: """ return list({key[1] for key in DATABASE['Isotopes']}) + def _resolve_structure_space_group(self) -> object | None: + """ + Return the parent structure's space-group category, or ``None``. + + Walks ``AtomSite`` → atom-sites collection → structure; returns + ``None`` when any link is missing (no parent context yet). + """ + collection = getattr(self, '_parent', None) + structure = getattr(collection, '_parent', None) if collection is not None else None + return getattr(structure, 'space_group', None) if structure is not None else None + @property def _wyckoff_letter_allowed_values(self) -> list[str]: """ - Return allowed Wyckoff-letter symbols. + Allowed Wyckoff letters for the current space group. Returns ------- list[str] - Currently a hard-coded placeholder list. - """ - # TODO: Need to now current space group. How to access it? Via - # parent Cell? Then letters = - # list(SPACE_GROUPS[62, 'cab']['Wyckoff_positions'].keys()) - # Temporarily return hardcoded list: - return ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i'] + ``['', *letters]`` for a tabulated space group (empty first, + so an unset letter is valid); ``[]`` when there is no parent + context or the space group is untabulated. + """ + space_group = self._resolve_structure_space_group() + if space_group is None: + return [] + positions = ecr.space_group_wyckoff_table( + space_group.name_h_m.value, + space_group.it_coordinate_system_code.value, + ) + if positions is None: + return [] + return ['', *positions] @property def _wyckoff_letter_default_value(self) -> str: @@ -220,10 +262,11 @@ def _wyckoff_letter_default_value(self) -> str: Returns ------- str - First element of the allowed values list. + The first allowed value (empty string), or ``''`` when no + letters are allowed. """ - # TODO: What to pass as default? - return self._wyckoff_letter_allowed_values[0] + allowed = self._wyckoff_letter_allowed_values + return allowed[0] if allowed else '' def _convert_adp_values(self, old_type: str, new_type: str) -> None: """ @@ -444,7 +487,34 @@ def wyckoff_letter(self) -> StringDescriptor: @wyckoff_letter.setter def wyckoff_letter(self, value: str) -> None: - self._wyckoff_letter.value = value + if self._resolve_structure_space_group() is None: + # No parent context yet (e.g. create() before the atom is + # added): store the raw value and defer validation to the + # update flow, which resolves it once context is available. + self._wyckoff_letter_needs_validation = True + self._wyckoff_letter._set_value_from_minimizer(value) + else: + self._wyckoff_letter.value = value + + @property + def multiplicity(self) -> IntegerDescriptor: + """ + Read-only site multiplicity derived from the Wyckoff position. + + Populated by Wyckoff detection; ``value`` is ``None`` when the + space group is untabulated. There is no public setter. + """ + return self._multiplicity + + def _set_wyckoff_letter_detected(self, letter: str) -> None: + """ + Set the auto-detected Wyckoff letter, bypassing validation. + + Modelled on ``_set_value_from_minimizer``: detection supplies a + trusted letter, written directly rather than re-validated + against the (dynamic) allowed-letters set. + """ + self._wyckoff_letter._set_value_from_minimizer(letter) @property def fract_x(self) -> Parameter: @@ -552,47 +622,150 @@ def __init__(self) -> None: # Private helper methods # ------------------------------------------------------------------ - def _apply_atomic_coordinates_symmetry_constraints(self) -> None: + def _apply_atomic_coordinates_symmetry_constraints( + self, *, called_by_minimizer: bool = False + ) -> None: """ - Apply symmetry rules to fractional coordinates of every site. + Detect Wyckoff letters and snap coordinates to symmetry. - Uses the parent structure's space-group symbol, IT coordinate - system code and each atom's Wyckoff letter. Atoms without a - Wyckoff letter are silently skipped. Coordinates fully - determined by site symmetry are flagged as - ``symmetry_constrained`` so they cannot be marked refinable. + For each atom: resolve any pending no-context Wyckoff letter; + (re)detect the letter when it is empty or the coordinates / + space-group key changed (skipped under a minimizer); snap + coordinates to the selected orbit representative; and record the + multiplicity and constrained-axis flags. Atoms in an untabulated + space group keep their stored letter unvalidated, with no + multiplicity or constraints. + + Parameters + ---------- + called_by_minimizer : bool, default=False + When True (per fit iteration), skip re-detection and + warnings; only the silent coordinate snap runs. """ structure = self._parent - space_group_name = structure.space_group.name_h_m.value - space_group_coord_code = structure.space_group.it_coordinate_system_code.value + name_hm = structure.space_group.name_h_m.value + coord_code = structure.space_group.it_coordinate_system_code.value + supported = ecr.space_group_wyckoff_table(name_hm, coord_code) is not None for atom in self._items: - wl = atom.wyckoff_letter.value - if not wl: - # TODO: Decide how to handle this case - self._clear_fract_symmetry_constrained(atom) - continue - dummy_atom = { - 'fract_x': atom.fract_x.value, - 'fract_y': atom.fract_y.value, - 'fract_z': atom.fract_z.value, - } - ecr.apply_atom_site_symmetry_constraints( - atom_site=dummy_atom, - name_hm=space_group_name, - coord_code=space_group_coord_code, - wyckoff_letter=wl, + if atom._wyckoff_letter_needs_validation: + self._resolve_pending_wyckoff_letter(atom, name_hm) + if supported: + self._detect_and_snap_atom( + atom, name_hm, coord_code, called_by_minimizer=called_by_minimizer + ) + else: + self._mark_atom_untabulated( + atom, (name_hm, coord_code), called_by_minimizer=called_by_minimizer + ) + + @staticmethod + def _resolve_pending_wyckoff_letter(atom: AtomSite, name_hm: str) -> None: + """ + Validate a deferred no-context Wyckoff letter; raise if invalid. + """ + stored = atom.wyckoff_letter.value + allowed = atom._wyckoff_letter_allowed_values + if allowed and stored not in allowed: + msg = ( + f'Invalid Wyckoff letter {stored!r} for space group ' + f'{name_hm!r}; allowed letters: {allowed}' ) - constrained_flags = ecr.atom_site_symmetry_constrained_flags( - name_hm=space_group_name, - coord_code=space_group_coord_code, - wyckoff_letter=wl, + raise ValueError(msg) + atom._wyckoff_letter_needs_validation = False + + def _mark_atom_untabulated( + self, + atom: AtomSite, + key: tuple[str, str | None], + *, + called_by_minimizer: bool, + ) -> None: + """Handle an atom whose space group is absent from the table.""" + atom._multiplicity.value = None + self._clear_fract_symmetry_constrained(atom) + if atom.wyckoff_letter.value and not called_by_minimizer: + log.warning( + f'Wyckoff letter of {atom.label.value} is stored but not ' + f'validated because the space group is untabulated' ) - atom.fract_x.value = dummy_atom['fract_x'] - atom.fract_y.value = dummy_atom['fract_y'] - atom.fract_z.value = dummy_atom['fract_z'] - atom._fract_x._set_symmetry_constrained(value=constrained_flags['fract_x']) - atom._fract_y._set_symmetry_constrained(value=constrained_flags['fract_y']) - atom._fract_z._set_symmetry_constrained(value=constrained_flags['fract_z']) + atom._wyckoff_coord_baseline = (atom.fract_x.value, atom.fract_y.value, atom.fract_z.value) + atom._wyckoff_key_baseline = key + + def _detect_and_snap_atom( + self, + atom: AtomSite, + name_hm: str, + coord_code: str | None, + *, + called_by_minimizer: bool, + ) -> None: + """ + Detect (if triggered) and snap one atom to its Wyckoff position. + """ + key = (name_hm, coord_code) + letter_before = atom.wyckoff_letter.value + coords = (atom.fract_x.value, atom.fract_y.value, atom.fract_z.value) + # A ``None`` baseline marks the first population + # (create/load), not a later edit. Treat coordinates or the + # space-group key as "changed" only against an existing + # baseline, so an explicit initial letter is preserved (routed + # to ``wyckoff_position_info`` below) instead of being + # overwritten by all-letter detection. The ADR requires a + # user-supplied letter to persist until a genuine later + # coordinate or space-group-key edit. + coords_changed = atom._wyckoff_coord_baseline is not None and any( + abs(a - b) > ecr._WYCKOFF_DETECTION_TOL + for a, b in zip(coords, atom._wyckoff_coord_baseline, strict=True) + ) + key_changed = atom._wyckoff_key_baseline is not None and atom._wyckoff_key_baseline != key + detect = (not called_by_minimizer) and (not letter_before or coords_changed or key_changed) + if detect: + position = ecr.detect_wyckoff_position(name_hm, coord_code, coords) + if position is not None and letter_before and position.letter != letter_before: + log.warning( + f'change moved the Wyckoff letter of {atom.label.value} ' + f'from {letter_before} to {position.letter}' + ) + if position is not None: + atom._set_wyckoff_letter_detected(position.letter) + elif letter_before: + position = ecr.wyckoff_position_info( + name_hm, coord_code, letter_before, fract_xyz=coords + ) + else: + position = None + + if position is None or position.coord_template is None: + atom._multiplicity.value = None + self._clear_fract_symmetry_constrained(atom) + atom._wyckoff_coord_baseline = coords + atom._wyckoff_key_baseline = key + return + + atom._multiplicity.value = position.multiplicity + snapped, flags = ecr.snap_to_wyckoff_template(position.coord_template, coords) + atom.fract_x.value = snapped[0] + atom.fract_y.value = snapped[1] + atom.fract_z.value = snapped[2] + atom._fract_x._set_symmetry_constrained(value=flags['fract_x']) + atom._fract_y._set_symmetry_constrained(value=flags['fract_y']) + atom._fract_z._set_symmetry_constrained(value=flags['fract_z']) + moved = any( + abs(s - c) > ecr._WYCKOFF_DETECTION_TOL for s, c in zip(snapped, coords, strict=True) + ) + if moved and not called_by_minimizer: + if not detect: + log.warning( + f'coordinates of {atom.label.value} did not fit letter ' + f'{position.letter} and were adjusted' + ) + elif letter_before and position.letter == letter_before: + log.warning( + f'coordinates of {atom.label.value} were adjusted to satisfy ' + f'Wyckoff letter {position.letter}' + ) + atom._wyckoff_coord_baseline = snapped + atom._wyckoff_key_baseline = key @staticmethod def _clear_fract_symmetry_constrained(atom: AtomSite) -> None: @@ -683,10 +856,11 @@ def _update( ---------- called_by_minimizer : bool, default=False Whether the update was triggered by the fitting minimizer. - Currently unused. + When True, Wyckoff re-detection and warnings are skipped; + only the silent coordinate snap runs. """ - del called_by_minimizer - - self._apply_atomic_coordinates_symmetry_constraints() + self._apply_atomic_coordinates_symmetry_constraints( + called_by_minimizer=called_by_minimizer + ) self._apply_adp_symmetry_constraints() self._sync_iso_from_aniso() diff --git a/src/easydiffraction/datablocks/structure/categories/space_group_wyckoff/__init__.py b/src/easydiffraction/datablocks/structure/categories/space_group_wyckoff/__init__.py new file mode 100644 index 000000000..3808d3075 --- /dev/null +++ b/src/easydiffraction/datablocks/structure/categories/space_group_wyckoff/__init__.py @@ -0,0 +1,9 @@ +# SPDX-FileCopyrightText: 2026 EasyScience contributors +# SPDX-License-Identifier: BSD-3-Clause + +from easydiffraction.datablocks.structure.categories.space_group_wyckoff.default import ( + SpaceGroupWyckoff, +) +from easydiffraction.datablocks.structure.categories.space_group_wyckoff.default import ( + SpaceGroupWyckoffCollection, +) diff --git a/src/easydiffraction/datablocks/structure/categories/space_group_wyckoff/default.py b/src/easydiffraction/datablocks/structure/categories/space_group_wyckoff/default.py new file mode 100644 index 000000000..3c6256567 --- /dev/null +++ b/src/easydiffraction/datablocks/structure/categories/space_group_wyckoff/default.py @@ -0,0 +1,206 @@ +# SPDX-FileCopyrightText: 2026 EasyScience contributors +# SPDX-License-Identifier: BSD-3-Clause +""" +Derived space-group Wyckoff category. + +Defines :class:`SpaceGroupWyckoff` items and the read-only +:class:`SpaceGroupWyckoffCollection`. Rows are derived from the bundled +space-group table for the structure's current space group; they are not +user-edited. ``Structure`` rebuilds the collection when the space group +changes via +:meth:`SpaceGroupWyckoffCollection._replace_from_space_group`. +""" + +from __future__ import annotations + +from typing import override + +from easydiffraction.core.category import CategoryCollection +from easydiffraction.core.category import CategoryItem +from easydiffraction.core.display_handler import DisplayHandler +from easydiffraction.core.metadata import TypeInfo +from easydiffraction.core.validation import AttributeSpec +from easydiffraction.core.variable import IntegerDescriptor +from easydiffraction.core.variable import StringDescriptor +from easydiffraction.crystallography import crystallography as ecr +from easydiffraction.datablocks.structure.categories.space_group_wyckoff.factory import ( + SpaceGroupWyckoffFactory, +) +from easydiffraction.io.cif.handler import CifHandler + +_READ_ONLY_MESSAGE = ( + 'space_group_wyckoff is derived from the space group and is read-only; ' + 'it is rebuilt automatically when the space group changes.' +) + + +class SpaceGroupWyckoff(CategoryItem): + """ + A single Wyckoff position of the space group (all fields read-only). + """ + + _category_code = 'space_group_Wyckoff' + _category_entry_name = 'id' + + def __init__(self) -> None: + """Initialise an empty Wyckoff-position row.""" + super().__init__() + + self._id = StringDescriptor( + name='id', + description='Identifier of the Wyckoff position.', + display_handler=DisplayHandler(display_name='ID', latex_name='ID'), + value_spec=AttributeSpec(default=''), + cif_handler=CifHandler(names=['_space_group_Wyckoff.id']), + ) + self._letter = StringDescriptor( + name='letter', + description='Wyckoff letter of the position.', + display_handler=DisplayHandler(display_name='Letter', latex_name='Letter'), + value_spec=AttributeSpec(default=''), + cif_handler=CifHandler(names=['_space_group_Wyckoff.letter']), + ) + self._multiplicity = IntegerDescriptor( + name='multiplicity', + description='Multiplicity of the Wyckoff position.', + display_handler=DisplayHandler(display_name='Multiplicity', latex_name='Multiplicity'), + value_spec=AttributeSpec(default=0), + cif_handler=CifHandler(names=['_space_group_Wyckoff.multiplicity']), + ) + self._site_symmetry = StringDescriptor( + name='site_symmetry', + description='Site-symmetry symbol of the Wyckoff position.', + display_handler=DisplayHandler( + display_name='Site symmetry', latex_name='Site symmetry' + ), + value_spec=AttributeSpec(default=''), + cif_handler=CifHandler(names=['_space_group_Wyckoff.site_symmetry']), + ) + self._coords_xyz = StringDescriptor( + name='coords_xyz', + description='Coordinates of the Wyckoff orbit.', + display_handler=DisplayHandler(display_name='Coordinates', latex_name='Coordinates'), + value_spec=AttributeSpec(default=''), + cif_handler=CifHandler(names=['_space_group_Wyckoff.coords_xyz']), + ) + + @property + def id(self) -> StringDescriptor: + """Read-only Wyckoff-position identifier.""" + return self._id + + @property + def letter(self) -> StringDescriptor: + """Read-only Wyckoff letter.""" + return self._letter + + @property + def multiplicity(self) -> IntegerDescriptor: + """Read-only multiplicity.""" + return self._multiplicity + + @property + def site_symmetry(self) -> StringDescriptor: + """Read-only site-symmetry symbol.""" + return self._site_symmetry + + @property + def coords_xyz(self) -> StringDescriptor: + """Read-only orbit coordinates.""" + return self._coords_xyz + + +@SpaceGroupWyckoffFactory.register +class SpaceGroupWyckoffCollection(CategoryCollection): + """ + Read-only collection of derived Wyckoff positions. + """ + + type_info = TypeInfo( + tag='default', + description='Derived space-group Wyckoff table', + ) + + def __init__(self) -> None: + """Initialise an empty derived Wyckoff collection.""" + super().__init__(item_type=SpaceGroupWyckoff) + + @override + def add(self, item: object) -> None: + """ + Reject public mutation; the collection is derived (read-only). + """ + raise ValueError(_READ_ONLY_MESSAGE) + + @override + def create(self, **kwargs: object) -> None: + """ + Reject public mutation; the collection is derived (read-only). + """ + raise ValueError(_READ_ONLY_MESSAGE) + + @override + def remove(self, name: str) -> None: + """ + Reject public mutation; the collection is derived (read-only). + """ + raise ValueError(_READ_ONLY_MESSAGE) + + @override + def __setitem__(self, name: str, item: object) -> None: + """ + Reject item assignment; the collection is derived (read-only). + """ + raise ValueError(_READ_ONLY_MESSAGE) + + @override + def __delitem__(self, name: str) -> None: + """ + Reject item deletion; the collection is derived (read-only). + """ + raise ValueError(_READ_ONLY_MESSAGE) + + @override + def from_cif(self, block: object) -> None: + """ + Ignore incoming CIF values for this derived category. + + The Wyckoff table is derived from the structure's space group + and is never read back from a CIF file: any + ``_space_group_Wyckoff.*`` loop in incoming CIF (for example a + hand-edited project file) is discarded and the table is rebuilt + from the space group on the next update. + """ + return + + def _replace_from_space_group(self) -> None: + """ + Rebuild rows from the parent structure's space group. + + Repopulates from the bundled Wyckoff table and adopts the new + rows via ``_adopt_items``, which rebuilds the name index and + parent links so a stale key lookup cannot survive a space-group + change. Leaves the collection empty for an absent/untabulated + space group. + """ + structure = getattr(self, '_parent', None) + if structure is None: + self._adopt_items([]) + return + name_hm = structure.space_group.name_h_m.value + coord_code = structure.space_group.it_coordinate_system_code.value + positions = ecr.space_group_wyckoff_table(name_hm, coord_code) + if not positions: + self._adopt_items([]) + return + rows = [] + for letter, position in positions.items(): + multiplicity = int(position['multiplicity']) + row = self._item_type() + row._id.value = f'{multiplicity}{letter}' + row._letter.value = letter + row._multiplicity.value = multiplicity + row._site_symmetry.value = str(position['site_symmetry']) + row._coords_xyz.value = ' '.join(position['coords_xyz']) + rows.append(row) + self._adopt_items(rows) diff --git a/src/easydiffraction/datablocks/structure/categories/space_group_wyckoff/factory.py b/src/easydiffraction/datablocks/structure/categories/space_group_wyckoff/factory.py new file mode 100644 index 000000000..2e3ea0980 --- /dev/null +++ b/src/easydiffraction/datablocks/structure/categories/space_group_wyckoff/factory.py @@ -0,0 +1,19 @@ +# SPDX-FileCopyrightText: 2026 EasyScience contributors +# SPDX-License-Identifier: BSD-3-Clause +""" +Space-group Wyckoff factory — delegates entirely to ``FactoryBase``. +""" + +from __future__ import annotations + +from typing import ClassVar + +from easydiffraction.core.factory import FactoryBase + + +class SpaceGroupWyckoffFactory(FactoryBase): + """Create space-group Wyckoff collections by tag.""" + + _default_rules: ClassVar[dict] = { + frozenset(): 'default', + } diff --git a/src/easydiffraction/datablocks/structure/item/base.py b/src/easydiffraction/datablocks/structure/item/base.py index 8d8d27928..704a98c9c 100644 --- a/src/easydiffraction/datablocks/structure/item/base.py +++ b/src/easydiffraction/datablocks/structure/item/base.py @@ -19,6 +19,12 @@ from easydiffraction.datablocks.structure.categories.geom.factory import GeomFactory from easydiffraction.datablocks.structure.categories.space_group import SpaceGroup from easydiffraction.datablocks.structure.categories.space_group.factory import SpaceGroupFactory +from easydiffraction.datablocks.structure.categories.space_group_wyckoff import ( + SpaceGroupWyckoffCollection, +) +from easydiffraction.datablocks.structure.categories.space_group_wyckoff.factory import ( + SpaceGroupWyckoffFactory, +) from easydiffraction.utils.logging import console from easydiffraction.utils.utils import render_cif @@ -43,6 +49,8 @@ def __init__( self._atom_site_aniso = AtomSiteAnisoFactory.create(self._atom_site_aniso_type) self._geom_type: str = GeomFactory.default_tag() self._geom = GeomFactory.create(self._geom_type) + self._space_group_wyckoff_type: str = SpaceGroupWyckoffFactory.default_tag() + self._space_group_wyckoff = SpaceGroupWyckoffFactory.create(self._space_group_wyckoff_type) self._identity.datablock_entry_name = lambda: self.name # ------------------------------------------------------------------ @@ -180,6 +188,13 @@ def geom(self, new: Geom) -> None: """ self._geom = new + @property + def space_group_wyckoff(self) -> SpaceGroupWyckoffCollection: + """ + Read-only Wyckoff table derived from the current space group. + """ + return self._space_group_wyckoff + # ------------------------------------------------------------------ # Private methods # ------------------------------------------------------------------ @@ -230,6 +245,7 @@ def _update_categories( if not called_by_minimizer and not self._need_categories_update: return + self._space_group_wyckoff._replace_from_space_group() self._sync_atom_site_aniso() for category in self.categories: @@ -237,6 +253,16 @@ def _update_categories( self._need_categories_update = False + def _serializable_categories(self) -> list: + """ + Project-CIF categories (excludes the derived Wyckoff table). + """ + return [ + category + for category in self.categories + if not isinstance(category, SpaceGroupWyckoffCollection) + ] + # ------------------------------------------------------------------ # Public methods # ------------------------------------------------------------------ diff --git a/src/easydiffraction/display/plotters/plotly.py b/src/easydiffraction/display/plotters/plotly.py index 61bd915c6..8bfed31a4 100644 --- a/src/easydiffraction/display/plotters/plotly.py +++ b/src/easydiffraction/display/plotters/plotly.py @@ -2305,24 +2305,25 @@ def _bragg_row_height_pixels(plot_spec: PowderMeasVsCalcSpec) -> float: def _baseline_non_bragg_row_heights( cls, plot_spec: PowderMeasVsCalcSpec, - row_count: int, *, - has_bragg_ticks: bool, has_residual: bool, ) -> tuple[float, float | None]: - """Return baseline main and residual row heights in pixels.""" + """ + Return fixed main and residual row heights in pixels. + + Anchored to the reference three-row layout so the main and + residual rows keep their pixel height regardless of which rows + are shown; ``_composite_figure_height`` adapts instead. + """ baseline_height = cls._base_composite_height_pixels(plot_spec) plot_area_height = cls._composite_plot_area_height(baseline_height) - available_row_pixels = plot_area_height * cls._subplot_available_height_fraction(row_count) - baseline_bragg_pixels = float( - cls._bragg_tick_symbol_height_pixels() if has_bragg_ticks else 0 - ) - non_bragg_pixels = max(available_row_pixels - baseline_bragg_pixels, 1.0) + available_row_pixels = plot_area_height * cls._subplot_available_height_fraction(3) + non_bragg_pixels = max(available_row_pixels - cls._bragg_tick_symbol_height_pixels(), 1.0) + main_pixels = non_bragg_pixels / (1.0 + plot_spec.residual_height_fraction) if not has_residual: - return non_bragg_pixels, None + return main_pixels, None - main_pixels = non_bragg_pixels / (1.0 + plot_spec.residual_height_fraction) residual_pixels = main_pixels * plot_spec.residual_height_fraction return main_pixels, residual_pixels @@ -2334,8 +2335,6 @@ def _get_powder_composite_rows(plot_spec: PowderMeasVsCalcSpec) -> PowderComposi row_count = 1 + int(has_bragg_ticks) + int(has_residual) main_row_height, residual_row_height = PlotlyPlotter._baseline_non_bragg_row_heights( plot_spec=plot_spec, - row_count=row_count, - has_bragg_ticks=has_bragg_ticks, has_residual=has_residual, ) row_heights = [main_row_height] @@ -2359,22 +2358,20 @@ def _get_powder_composite_rows(plot_spec: PowderMeasVsCalcSpec) -> PowderComposi ) @classmethod - def _composite_figure_height( - cls, - plot_spec: PowderMeasVsCalcSpec, - layout: PowderCompositeRows, - ) -> float: - """Return figure height for Bragg row growth.""" - base_pixels = cls._base_composite_height_pixels(plot_spec) - phase_count = len(plot_spec.bragg_tick_sets) - if phase_count <= 1: - return base_pixels - - added_bragg_pixels = float((phase_count - 1) * cls._bragg_tick_symbol_height_pixels()) - growth_pixels = added_bragg_pixels / cls._subplot_available_height_fraction( - layout.row_count - ) - return base_pixels + growth_pixels + def _composite_figure_height(cls, layout: PowderCompositeRows) -> float: + """ + Return figure height matching the row pixel heights. + + Each entry in ``layout.row_heights`` is an absolute pixel + target. Plotly distributes the plot area across rows by + fraction, so the figure height is the row-pixel sum scaled up + for the inter-row spacing, plus the vertical margins. The main + and residual rows stay fixed while the Bragg row (and the + figure) grow with the phase count. + """ + row_pixels = sum(layout.row_heights) + plot_area_height = row_pixels / cls._subplot_available_height_fraction(layout.row_count) + return plot_area_height + COMPOSITE_MARGIN_TOP + COMPOSITE_MARGIN_BOTTOM @classmethod def _get_main_intensity_range(cls, plot_spec: PowderMeasVsCalcSpec) -> tuple[float, float]: @@ -2698,7 +2695,7 @@ def _configure_powder_composite_layout( layout: PowderCompositeRows, ) -> None: fig.update_layout( - height=self._composite_figure_height(plot_spec, layout), + height=self._composite_figure_height(layout), margin={ 'autoexpand': True, 'r': COMPOSITE_MARGIN_RIGHT, diff --git a/src/easydiffraction/io/cif/iucr_writer.py b/src/easydiffraction/io/cif/iucr_writer.py index b0d5d9513..6db8aae8f 100644 --- a/src/easydiffraction/io/cif/iucr_writer.py +++ b/src/easydiffraction/io/cif/iucr_writer.py @@ -171,6 +171,7 @@ def _write_sc_block( _write_cell_section(lines, structure) _write_space_group_section(lines, structure) _write_symmetry_operations_section(lines, structure) + _write_space_group_wyckoff_section(lines, structure) _write_diffrn_section(lines, experiment) _write_wavelength_section(lines, experiment) _write_atom_site_sections(lines, structure) @@ -222,6 +223,53 @@ def _write_symmetry_operations_section(lines: list[str], structure: object) -> N _write_loop(lines, loop.tags, loop.rows) +def _write_space_group_wyckoff_section(lines: list[str], structure: object) -> None: + """ + Append the derived space-group Wyckoff-position loop. + + This loop is report-only: it summarises every Wyckoff position of + the structure's space group. ``coords_xyz`` reports the + representative orbit coordinate (the first orbit member) to keep + loop cells compact. + """ + positions = list(_collection_values(getattr(structure, 'space_group_wyckoff', None))) + if not positions: + return + + rows = [ + ( + _attribute_descriptor(position, 'id'), + _attribute_descriptor(position, 'letter'), + _attribute_descriptor(position, 'multiplicity'), + _attribute_descriptor(position, 'site_symmetry'), + _wyckoff_representative_coord(position), + ) + for position in positions + ] + _section(lines, 'Wyckoff positions') + _write_loop( + lines, + ( + '_space_group_Wyckoff.id', + '_space_group_Wyckoff.letter', + '_space_group_Wyckoff.multiplicity', + '_space_group_Wyckoff.site_symmetry', + '_space_group_Wyckoff.coords_xyz', + ), + rows, + ) + + +def _wyckoff_representative_coord(position: object) -> str: + """ + Return the representative (first) orbit coordinate of a position. + """ + coords = _attribute_value(position, 'coords_xyz') + if not coords: + return '?' + return str(coords).split()[0] + + def _write_diffrn_section(lines: list[str], experiment: object) -> None: """Append diffraction metadata.""" diffrn = getattr(experiment, 'diffrn', None) @@ -449,6 +497,7 @@ def _write_powder_phase_block(phase: _PowderPhase) -> str: _write_cell_section(lines, phase.structure) _write_space_group_section(lines, phase.structure) _write_symmetry_operations_section(lines, phase.structure) + _write_space_group_wyckoff_section(lines, phase.structure) _write_atom_site_sections(lines, phase.structure) _write_atom_site_aniso_sections(lines, phase.structure) _write_powder_phase_reference_section(lines, phase) @@ -874,6 +923,7 @@ def _atom_site_tags(family: str) -> tuple[str, ...]: '_atom_site.ADP_type', f'_atom_site.{family}_iso_or_equiv', '_atom_site.Wyckoff_symbol', + '_atom_site.site_symmetry_multiplicity', ) @@ -889,6 +939,7 @@ def _atom_site_row(atom_site: object) -> tuple[object, ...]: _attribute_descriptor(atom_site, 'adp_type'), _attribute_descriptor(atom_site, 'adp_iso'), _attribute_descriptor(atom_site, 'wyckoff_letter'), + _attribute_descriptor(atom_site, 'multiplicity'), ) diff --git a/src/easydiffraction/report/fit_plot.py b/src/easydiffraction/report/fit_plot.py index 1f60fc677..3ffe55df0 100644 --- a/src/easydiffraction/report/fit_plot.py +++ b/src/easydiffraction/report/fit_plot.py @@ -105,16 +105,19 @@ def fit_plot_ranges(fit_data: dict[str, Any]) -> dict[str, float]: def fit_plot_geometry(fit_data: dict[str, Any]) -> dict[str, float]: - """Return Plotly-matched pgfplots axis geometry.""" + """ + Return Plotly-matched pgfplots axis geometry. + + Row heights are anchored to the reference three-row layout so the + main and residual panels keep a fixed centimetre height; the axis + stack grows or shrinks with the rows shown, matching the interactive + composite figure. + """ bragg_tick_sets = fit_data.get('bragg_tick_sets') or [] has_bragg_ticks = bool(bragg_tick_sets) has_residual = _has_residual(fit_data) row_count = 1 + int(has_bragg_ticks) + int(has_residual) - main_pixels, residual_pixels = _non_bragg_row_heights( - row_count=row_count, - has_bragg_ticks=has_bragg_ticks, - has_residual=has_residual, - ) + main_pixels, residual_pixels = _non_bragg_row_heights(has_residual=has_residual) row_heights = [main_pixels] if has_bragg_ticks: @@ -122,16 +125,15 @@ def fit_plot_geometry(fit_data: dict[str, Any]) -> dict[str, float]: if has_residual and residual_pixels is not None: row_heights.append(residual_pixels) - height_sum = sum(row_heights) - stack_height = _FIGURE_AXIS_WIDTH_CM * _FIGURE_AXIS_HEIGHT_TO_WIDTH - row_area_height = stack_height * _subplot_available_height_fraction(row_count) - scaled_heights = [row_area_height * row_height / height_sum for row_height in row_heights] + cm_per_pixel = _figure_cm_per_pixel() + row_heights_cm = [row_height * cm_per_pixel for row_height in row_heights] + plot_area_cm = sum(row_heights_cm) / _subplot_available_height_fraction(row_count) return { 'axis_width_cm': _FIGURE_AXIS_WIDTH_CM, - 'main_height_cm': scaled_heights[0], - 'bragg_height_cm': scaled_heights[1] if has_bragg_ticks else 0.0, - 'residual_height_cm': scaled_heights[-1] if has_residual else 0.0, - 'vertical_sep_cm': _vertical_sep_cm(stack_height), + 'main_height_cm': row_heights_cm[0], + 'bragg_height_cm': row_heights_cm[1] if has_bragg_ticks else 0.0, + 'residual_height_cm': row_heights_cm[-1] if has_residual else 0.0, + 'vertical_sep_cm': _vertical_sep_cm(plot_area_cm), } @@ -234,25 +236,36 @@ def _has_residual(fit_data: dict[str, Any]) -> bool: return isinstance(series, dict) and 'diff' in series -def _non_bragg_row_heights( - *, - row_count: int, - has_bragg_ticks: bool, - has_residual: bool, -) -> tuple[float, float | None]: +def _non_bragg_row_heights(*, has_residual: bool) -> tuple[float, float | None]: + """ + Return fixed main and residual row heights in pixels. + + Anchored to the reference three-row layout so the rows keep a + constant height regardless of which rows the figure shows. + """ plot_area_height = _composite_plot_area_height() - available_row_pixels = plot_area_height * _subplot_available_height_fraction(row_count) - baseline_bragg_pixels = _bragg_tick_symbol_height_pixels() if has_bragg_ticks else 0.0 - non_bragg_pixels = max(available_row_pixels - baseline_bragg_pixels, 1.0) + available_row_pixels = plot_area_height * _subplot_available_height_fraction(3) + non_bragg_pixels = max(available_row_pixels - _bragg_tick_symbol_height_pixels(), 1.0) + main_pixels = non_bragg_pixels / (1.0 + DEFAULT_RESIDUAL_HEIGHT_FRACTION) if not has_residual: - return non_bragg_pixels, None + return main_pixels, None - main_pixels = non_bragg_pixels / (1.0 + DEFAULT_RESIDUAL_HEIGHT_FRACTION) residual_pixels = main_pixels * DEFAULT_RESIDUAL_HEIGHT_FRACTION return main_pixels, residual_pixels +def _figure_cm_per_pixel() -> float: + """ + Return the fixed cm-per-pixel scale for fit-figure rows. + + Anchored so the reference three-row layout fills the nominal axis + stack height (axis width times the reference aspect ratio). + """ + reference_stack_cm = _FIGURE_AXIS_WIDTH_CM * _FIGURE_AXIS_HEIGHT_TO_WIDTH + return reference_stack_cm / _composite_plot_area_height() + + def _composite_plot_area_height() -> float: full_height = float(DEFAULT_HEIGHT * PLOTLY_HEIGHT_PER_UNIT) return max(full_height - COMPOSITE_MARGIN_TOP - COMPOSITE_MARGIN_BOTTOM, 1.0) @@ -270,8 +283,8 @@ def _bragg_row_height_pixels(tick_set_count: int) -> float: return float(tick_set_count) * _bragg_tick_symbol_height_pixels() -def _vertical_sep_cm(stack_height: float) -> float: - return stack_height * COMPOSITE_VERTICAL_SPACING +def _vertical_sep_cm(plot_area_cm: float) -> float: + return plot_area_cm * COMPOSITE_VERTICAL_SPACING def _display_tick_limit(raw_limit: float) -> float: diff --git a/tests/functional/test_wyckoff_tutorial_corpus.py b/tests/functional/test_wyckoff_tutorial_corpus.py new file mode 100644 index 000000000..08306c269 --- /dev/null +++ b/tests/functional/test_wyckoff_tutorial_corpus.py @@ -0,0 +1,166 @@ +# SPDX-FileCopyrightText: 2026 EasyScience contributors +# SPDX-License-Identifier: BSD-3-Clause +"""Tutorial-corpus regression for Wyckoff-letter detection. + +Each tutorial structure has a known Wyckoff letter for every site, so the +letter is ground truth. This test re-derives the letter from the site's +coordinates and space group and asserts it matches the known one -- broad, +real-world coverage that also guards against regressions whenever the +tutorials change. + +Coverage comes from two sources: + +1. Tutorials that still declare ``wyckoff_letter`` explicitly are parsed + statically (no execution / fitting) and checked directly. +2. Many tutorials were switched to rely on auto-detection (the explicit + letters were removed from the source). Their original declarations are + preserved here as an explicit ``_GROUND_TRUTH`` table so the regression + still exercises those structures -- including the R-3m coupled special + position ``(x, -x, z)`` from ed-6 that motivated the canonical-template + work. +""" + +import ast +import pathlib + +from easydiffraction.crystallography import crystallography as ecr + +_TUTORIALS_DIR = pathlib.Path(__file__).resolve().parents[2] / 'docs' / 'docs' / 'tutorials' + +# Known (space group, coordinate code, fractional coordinates, Wyckoff +# letter) for tutorial sites that no longer declare the letter in source +# (they now exercise auto-detection). Detection must reproduce each letter. +_GROUND_TRUTH = [ + ('F d -3 m', '2', (0.125, 0.125, 0.125), 'a'), + ('F m -3 m', '1', (0.0, 0.0, 0.0), 'a'), + ('F m -3 m', '1', (0.5, 0.5, 0.5), 'b'), + ('I 21 3', '1', (0.0851, 0.0851, 0.0851), 'a'), + ('I 21 3', '1', (0.1377, 0.3054, 0.1195), 'c'), + ('I 21 3', '1', (0.2521, 0.2521, 0.2521), 'a'), + ('I 21 3', '1', (0.3625, 0.3633, 0.1867), 'c'), + ('I 21 3', '1', (0.4612, 0.4612, 0.4612), 'a'), + ('I 21 3', '1', (0.4663, 0.0, 0.25), 'b'), + ('P m -3 m', '1', (0.0, 0.0, 0.0), 'a'), + ('P m -3 m', '1', (0.0, 0.5, 0.5), 'c'), + ('P m -3 m', '1', (0.5, 0.5, 0.5), 'b'), + ('P n m a', 'abc', (0.0, 0.0, 0.0), 'a'), + ('P n m a', 'abc', (0.0654, 0.25, 0.684), 'c'), + ('P n m a', 'abc', (0.0811, 0.0272, 0.8086), 'd'), + ('P n m a', 'abc', (0.091, 0.25, 0.771), 'c'), + ('P n m a', 'abc', (0.094, 0.25, 0.429), 'c'), + ('P n m a', 'abc', (0.164, 0.032, 0.28), 'd'), + ('P n m a', 'abc', (0.1876, 0.25, 0.167), 'c'), + ('P n m a', 'abc', (0.1935, 0.25, 0.5432), 'c'), + ('P n m a', 'abc', (0.279, 0.25, 0.985), 'c'), + ('P n m a', 'abc', (0.448, 0.25, 0.217), 'c'), + ('P n m a', 'abc', (0.9082, 0.25, 0.5954), 'c'), + ('R -3 m', 'h', (0.0, 0.0, 0.197), 'c'), + ('R -3 m', 'h', (0.0, 0.0, 0.5), 'b'), + ('R -3 m', 'h', (0.13, -0.13, 0.08), 'h'), + ('R -3 m', 'h', (0.21, -0.21, 0.06), 'h'), + ('R -3 m', 'h', (0.5, 0.0, 0.0), 'e'), +] + + +def _const(node): + """Return a literal value for a Constant or a negated Constant. + + Returns ``None`` for any other node so non-literal arguments are + ignored. Handles negative numeric literals such as ``-0.21``, which + parse as ``UnaryOp(USub, Constant)`` rather than a bare ``Constant``. + """ + if isinstance(node, ast.Constant): + return node.value + if ( + isinstance(node, ast.UnaryOp) + and isinstance(node.op, ast.USub) + and isinstance(node.operand, ast.Constant) + ): + return -node.operand.value + return None + + +def _string_assignment(tree, attr_name): + """Return the unique str assigned to ``*.`` or ``None``. + + Returns ``None`` when the attribute is assigned zero or several + times, so callers can skip ambiguous (multi-structure) tutorials. + """ + values = [ + _const(node.value) + for node in ast.walk(tree) + if isinstance(node, ast.Assign) and isinstance(_const(node.value), str) + for target in node.targets + if isinstance(target, ast.Attribute) and target.attr == attr_name + ] + if len(values) == 1: + return values[0] + return None + + +def _wyckoff_declarations(tree): + """Yield ``(label, letter, (x, y, z))`` for create() calls with a letter.""" + for node in ast.walk(tree): + if not (isinstance(node, ast.Call) and isinstance(node.func, ast.Attribute)): + continue + if node.func.attr != 'create': + continue + kwargs = { + kw.arg: _const(kw.value) + for kw in node.keywords + if kw.arg is not None and _const(kw.value) is not None + } + if 'wyckoff_letter' not in kwargs: + continue + coords = ( + float(kwargs.get('fract_x', 0.0)), + float(kwargs.get('fract_y', 0.0)), + float(kwargs.get('fract_z', 0.0)), + ) + yield kwargs.get('label', '?'), kwargs['wyckoff_letter'], coords + + +def _corpus(): + """Collect ``(tutorial, name_hm, code, label, letter, coords)`` rows.""" + rows = [] + for path in sorted(_TUTORIALS_DIR.glob('*.py')): + tree = ast.parse(path.read_text(encoding='utf-8')) + name_hm = _string_assignment(tree, 'name_h_m') + if name_hm is None: + # No structure, or several structures we cannot unambiguously + # associate with create() calls: skip this tutorial. + continue + code = _string_assignment(tree, 'it_coordinate_system_code') + for label, letter, coords in _wyckoff_declarations(tree): + rows.append((path.name, name_hm, code, label, letter, coords)) + return rows + + +def test_tutorial_declared_letters_are_reproduced_by_detection(): + rows = _corpus() + # Guard against a silently empty corpus (e.g. parser drift): some + # tutorials still declare explicit Wyckoff letters. + assert rows, 'no tutorial Wyckoff-letter declarations were found' + + mismatches = [] + for tutorial, name_hm, code, label, letter, coords in rows: + detected = ecr.detect_wyckoff_position(name_hm, code, coords) + if detected is None or detected.letter != letter: + found = None if detected is None else detected.letter + mismatches.append((tutorial, label, name_hm, coords, letter, found)) + + assert not mismatches, mismatches + + +def test_ground_truth_letters_are_reproduced_by_detection(): + # Structures whose explicit letters were removed in favour of + # auto-detection are covered here so the regression still exercises + # them (notably the R-3m coupled special position). + mismatches = [] + for name_hm, code, coords, letter in _GROUND_TRUTH: + detected = ecr.detect_wyckoff_position(name_hm, code, coords) + if detected is None or detected.letter != letter: + found = None if detected is None else detected.letter + mismatches.append((name_hm, code, coords, letter, found)) + + assert not mismatches, mismatches diff --git a/tests/integration/fitting/test_powder-diffraction_constant-wavelength.py b/tests/integration/fitting/test_powder-diffraction_constant-wavelength.py index d4c22b857..17f84be3d 100644 --- a/tests/integration/fitting/test_powder-diffraction_constant-wavelength.py +++ b/tests/integration/fitting/test_powder-diffraction_constant-wavelength.py @@ -478,6 +478,20 @@ def test_fit_neutron_pd_cwl_hs() -> None: decimal=1, ) + # ed-6 regression: O and H sit on R-3m 'h' = (x,-x,z), so after freeing + # and refining fract_x, fract_y must stay slaved to -fract_x (on-site). + # Operator-form coords_xyz wrongly freed fract_y and let them drift. + assert_almost_equal( + model.atom_sites['O'].fract_y.value, + desired=-model.atom_sites['O'].fract_x.value, + decimal=6, + ) + assert_almost_equal( + model.atom_sites['H'].fract_y.value, + desired=-model.atom_sites['H'].fract_x.value, + decimal=6, + ) + def test_single_fit_neutron_pd_cwl_lbco_with_constraints_from_project(tmp_path) -> None: import easydiffraction as ed diff --git a/tests/unit/easydiffraction/analysis/calculators/test_cryspy.py b/tests/unit/easydiffraction/analysis/calculators/test_cryspy.py index 6cebe4c33..c2a268c95 100644 --- a/tests/unit/easydiffraction/analysis/calculators/test_cryspy.py +++ b/tests/unit/easydiffraction/analysis/calculators/test_cryspy.py @@ -101,6 +101,11 @@ def test_update_structure_restores_wyckoff_multiplicity_after_coordinate_wrappin wyckoff_letter='h', adp_iso=0.5, ) + # The calculator reads the per-site multiplicity from the model, so + # run the update flow to populate it via Wyckoff detection (R-3m 'h' + # has multiplicity 18). + structure._update_categories() + assert structure.atom_sites['O'].multiplicity.value == 18 cryspy_model_dict = { 'unit_cell_parameters': [6.86, 6.86, 14.14, np.pi / 2, np.pi / 2, 2 * np.pi / 3], diff --git a/tests/unit/easydiffraction/crystallography/test_crystallography_wyckoff.py b/tests/unit/easydiffraction/crystallography/test_crystallography_wyckoff.py index 377ed7f2f..386b4cb55 100644 --- a/tests/unit/easydiffraction/crystallography/test_crystallography_wyckoff.py +++ b/tests/unit/easydiffraction/crystallography/test_crystallography_wyckoff.py @@ -14,13 +14,15 @@ def test_invalid_name_hm_returns_none(self, monkeypatch): assert result is None monkeypatch.setattr(Logger, '_reaction', Logger.Reaction.RAISE, raising=True) - def test_none_coord_code_returns_none(self, monkeypatch): + def test_none_coord_code_resolves_triclinic(self): from easydiffraction.crystallography.crystallography import _get_wyckoff_exprs - monkeypatch.setattr(Logger, '_reaction', Logger.Reaction.WARN, raising=True) + # Triclinic groups are keyed ``(IT_number, None)``: P 1 (IT 1) + # resolves through the ``None`` coordinate code to its general + # position (x, y, z) rather than being treated as unset. result = _get_wyckoff_exprs('P 1', None, 'a') - assert result is None - monkeypatch.setattr(Logger, '_reaction', Logger.Reaction.RAISE, raising=True) + assert result is not None + assert len(result) == 3 def test_valid_returns_three_expressions(self): from easydiffraction.crystallography.crystallography import _get_wyckoff_exprs @@ -54,6 +56,20 @@ def test_valid_applies_constraints(self): result = apply_atom_site_symmetry_constraints(atom, 'P m -3 m', '1', 'a') assert result is not None + def test_coupled_special_position_slaves_dependent_axis(self): + from easydiffraction.crystallography.crystallography import ( + apply_atom_site_symmetry_constraints, + ) + + # R -3 m (IT 166), coord_code='h', Wyckoff 'h' = (x,-x,z): editing + # fract_x slaves fract_y to -fract_x while fract_x/fract_z stay free + # (the ed-6 coupled-position regression). + atom = {'fract_x': 0.3, 'fract_y': 0.0, 'fract_z': 0.5} + result = apply_atom_site_symmetry_constraints(atom, 'R -3 m', 'h', 'h') + assert result['fract_x'] == 0.3 + assert result['fract_y'] == -0.3 + assert result['fract_z'] == 0.5 + class TestAtomSiteSymmetryConstrainedFlags: def test_special_position_all_fixed(self): @@ -74,6 +90,17 @@ def test_general_position_all_free(self): flags = atom_site_symmetry_constrained_flags('P 1', '1', 'a') assert flags == {'fract_x': False, 'fract_y': False, 'fract_z': False} + def test_coupled_special_position_constrains_dependent_axis(self): + from easydiffraction.crystallography.crystallography import ( + atom_site_symmetry_constrained_flags, + ) + + # R -3 m (IT 166), Wyckoff 'h' = (x,-x,z): fract_y is slaved to -x, + # so only fract_y is constrained. Operator-form coords_xyz would + # wrongly mark fract_y free (the canonical-templates regression). + flags = atom_site_symmetry_constrained_flags('R -3 m', 'h', 'h') + assert flags == {'fract_x': False, 'fract_y': True, 'fract_z': False} + def test_invalid_returns_all_false(self, monkeypatch): from easydiffraction.crystallography.crystallography import ( atom_site_symmetry_constrained_flags, @@ -84,3 +111,91 @@ def test_invalid_returns_all_false(self, monkeypatch): flags = atom_site_symmetry_constrained_flags('NOT REAL', None, 'a') assert flags == {'fract_x': False, 'fract_y': False, 'fract_z': False} monkeypatch.setattr(Logger, '_reaction', Logger.Reaction.RAISE, raising=True) + + +class TestDetectWyckoffPosition: + def test_general_position(self): + from easydiffraction.crystallography.crystallography import detect_wyckoff_position + + # A generic point in P m -3 m is the general position 'n' (48). + position = detect_wyckoff_position('P m -3 m', '1', (0.12, 0.23, 0.34)) + assert position.letter == 'n' + assert position.multiplicity == 48 + + def test_special_position(self): + from easydiffraction.crystallography.crystallography import detect_wyckoff_position + + position = detect_wyckoff_position('P m -3 m', '1', (0.0, 0.0, 0.0)) + assert position.letter == 'a' + assert position.multiplicity == 1 + + def test_multiplicity_tie_break_prefers_most_special(self): + from easydiffraction.crystallography.crystallography import detect_wyckoff_position + + # (1/2,0,0) lies on both 'e' = (x,0,0) (mult 6) and the more + # special 'd' = (1/2,0,0) (mult 3); detection prefers 'd'. + position = detect_wyckoff_position('P m -3 m', '1', (0.5, 0.0, 0.0)) + assert position.letter == 'd' + assert position.multiplicity == 3 + + def test_non_first_orbit_representative(self): + from easydiffraction.crystallography.crystallography import detect_wyckoff_position + + # (0,y,0) is on the 'e' orbit via a non-first representative + # ((0,x,0), not the tabulated first rep (x,0,0)). + position = detect_wyckoff_position('P m -3 m', '1', (0.0, 0.3, 0.0)) + assert position.letter == 'e' + + def test_rounded_input_matches_within_tolerance(self): + from easydiffraction.crystallography.crystallography import detect_wyckoff_position + + # x = 0.3333 ~ 1/3 is still on 'e' = (x,0,0) at the 1e-3 tolerance. + position = detect_wyckoff_position('P m -3 m', '1', (0.3333, 0.0, 0.0)) + assert position.letter == 'e' + + def test_empty_coord_code_normalises_to_none(self): + from easydiffraction.crystallography.crystallography import detect_wyckoff_position + + # P 1 is keyed (1, None); an empty coordinate code resolves there. + position = detect_wyckoff_position('P 1', '', (0.1, 0.2, 0.3)) + assert position is not None + assert position.letter == 'a' + + def test_absent_group_returns_none(self, monkeypatch): + from easydiffraction.crystallography.crystallography import detect_wyckoff_position + + monkeypatch.setattr(Logger, '_reaction', Logger.Reaction.WARN, raising=True) + assert detect_wyckoff_position('NOT A REAL SG', None, (0.1, 0.2, 0.3)) is None + monkeypatch.setattr(Logger, '_reaction', Logger.Reaction.RAISE, raising=True) + + +class TestWyckoffPositionInfo: + def test_without_coords_has_no_template(self): + from easydiffraction.crystallography.crystallography import wyckoff_position_info + + position = wyckoff_position_info('P m -3 m', '1', 'e') + assert position.letter == 'e' + assert position.multiplicity == 6 + assert position.coord_template is None + + def test_selects_nearest_representative_not_first(self): + from easydiffraction.crystallography.crystallography import ( + snap_to_wyckoff_template, + wyckoff_position_info, + ) + + # 'e' first rep is (x,0,0); for a point near the (0,x,0) member the + # nearest representative must be chosen so the snap keeps fract_y + # free near 0.3 instead of collapsing onto (x,0,0) -> (0,0,0). + position = wyckoff_position_info('P m -3 m', '1', 'e', fract_xyz=(0.0, 0.3, 0.0)) + assert position.coord_template is not None + snapped, _flags = snap_to_wyckoff_template(position.coord_template, (0.0, 0.3, 0.0)) + assert abs(snapped[0]) < 1e-6 + assert abs(snapped[1] - 0.3) < 1e-6 + assert abs(snapped[2]) < 1e-6 + + def test_absent_letter_returns_none(self): + from easydiffraction.crystallography.crystallography import wyckoff_position_info + + # P m -3 m has no Wyckoff letter 'z'. + assert wyckoff_position_info('P m -3 m', '1', 'z') is None diff --git a/tests/unit/easydiffraction/crystallography/test_space_groups.py b/tests/unit/easydiffraction/crystallography/test_space_groups.py index 37eab7235..4d085a8b1 100644 --- a/tests/unit/easydiffraction/crystallography/test_space_groups.py +++ b/tests/unit/easydiffraction/crystallography/test_space_groups.py @@ -2,6 +2,8 @@ # SPDX-License-Identifier: BSD-3-Clause """Unit tests for the space-group reference-data loader.""" +import re + from easydiffraction.crystallography.space_groups import SPACE_GROUPS _EXPECTED_RECORD_KEYS = { @@ -24,6 +26,11 @@ # runtime coordinate-code aliases. A deliberate regeneration updates this. _EXPECTED_RECORD_COUNT = 816 +# Canonical coords_xyz forbids operator form (``1/2*x``) and fractional +# coefficients (``5/4x``); integer coefficients (``2x``) and rational +# constants (``x+1/2``) are allowed. +_NONCANONICAL_COORD = re.compile(r'[0-9.]\s*\*\s*[xyz]|[xyz]\s*\*|\d+/\d+\s*[xyz]') + def test_module_import(): import easydiffraction.crystallography.space_groups as MUT @@ -72,3 +79,51 @@ def test_every_record_has_the_expected_schema(): assert isinstance(position['multiplicity'], int) assert isinstance(position['coords_xyz'], list) assert position['coords_xyz'] + + +def test_coords_xyz_are_canonical_distinct_full_orbits(): + """Every Wyckoff coords_xyz is a canonical, distinct, full orbit. + + Regression guard for the canonical-templates fix: no operator-form or + fractional-coefficient spelling, no duplicate template, and a length + equal to the multiplicity (the full centered orbit). + """ + for key, record in SPACE_GROUPS.items(): + for letter, position in record['Wyckoff_positions'].items(): + coords = position['coords_xyz'] + for template in coords: + assert not _NONCANONICAL_COORD.search(template), (key, letter, template) + assert len(set(coords)) == len(coords), (key, letter) + assert len(coords) == position['multiplicity'], (key, letter) + + +def test_representative_template_has_no_self_axis_coupling(): + """Every Wyckoff representative is in canonical parametric form. + + For the representative ``coords_xyz[0]``, no component may couple its + own axis variable with another axis (e.g. an x-slot term ``x+y``): + that is operator-form leakage, the ed-6 regression that the + canonical-templates fix removed. The slot-based constraint logic in + crystallography.py relies on this canonical form, so it is asserted + from the data side to block a silent reintroduction on a future + table regeneration. Non-first orbit members are genuine symmetry + images and may legitimately couple, so only the representative is + checked. + """ + from easydiffraction.crystallography.crystallography import _parse_rotation_matrix + + for key, record in SPACE_GROUPS.items(): + for letter, position in record['Wyckoff_positions'].items(): + representative = position['coords_xyz'][0] + rot, _trans = _parse_rotation_matrix(representative) + for axis_index in range(3): + references_own_axis = rot[axis_index, axis_index] != 0 + couples_other_axis = any( + rot[axis_index, other] != 0 for other in range(3) if other != axis_index + ) + assert not (references_own_axis and couples_other_axis), ( + key, + letter, + representative, + axis_index, + ) diff --git a/tests/unit/easydiffraction/datablocks/structure/categories/test_atom_sites.py b/tests/unit/easydiffraction/datablocks/structure/categories/test_atom_sites.py index 17d0dd5f5..2958fa494 100644 --- a/tests/unit/easydiffraction/datablocks/structure/categories/test_atom_sites.py +++ b/tests/unit/easydiffraction/datablocks/structure/categories/test_atom_sites.py @@ -112,10 +112,16 @@ def test_type_symbol_allowed_values(self): assert 'Fe' in allowed def test_wyckoff_letter_allowed_values(self): - from easydiffraction.datablocks.structure.categories.atom_sites.default import AtomSite + from easydiffraction.datablocks.structure.item.base import Structure - site = AtomSite() - allowed = site._wyckoff_letter_allowed_values + # Allowed letters are derived from the parent structure's space + # group, so the atom must live inside a structure with a + # tabulated space group; a parentless AtomSite has no allowed + # letters. + structure = Structure(name='s') + structure.space_group.name_h_m = 'P m -3 m' + structure.atom_sites.create(label='X', type_symbol='O', adp_iso=0.5) + allowed = structure.atom_sites['X']._wyckoff_letter_allowed_values assert 'a' in allowed def test_uses_iucr_casing_with_legacy_aliases(self): @@ -262,3 +268,152 @@ def test_uani_as_b_converts(self): structure.atom_sites['Si'].adp_type = 'Uani' expected_b = u_val * 8.0 * math.pi**2 assert math.isclose(structure.atom_sites['Si'].adp_iso_as_b, expected_b, rel_tol=1e-10) + + +# ------------------------------------------------------------------ +# Wyckoff letter detection / multiplicity +# ------------------------------------------------------------------ + + +class TestAtomSiteWyckoffDetection: + @staticmethod + def _structure(name_hm='P m -3 m'): + from easydiffraction.datablocks.structure.item.base import Structure + + structure = Structure(name='s') + structure.space_group.name_h_m = name_hm + return structure + + def test_fill_if_empty_on_update(self): + structure = self._structure() + structure.atom_sites.create(label='A', type_symbol='O', adp_iso=0.5) + structure._update_categories() + atom = structure.atom_sites['A'] + assert atom.wyckoff_letter.value == 'a' + assert atom.multiplicity.value == 1 + + def test_redetect_via_property_setter(self): + structure = self._structure() + structure.atom_sites.create(label='A', type_symbol='O', adp_iso=0.5) + structure._update_categories() + structure.atom_sites['A'].fract_x = 0.3 + structure._update_categories() + assert structure.atom_sites['A'].wyckoff_letter.value == 'e' + + def test_redetect_via_descriptor_value(self): + structure = self._structure() + structure.atom_sites.create(label='A', type_symbol='O', adp_iso=0.5) + structure._update_categories() + structure.atom_sites['A'].fract_x.value = 0.3 + structure._update_categories() + assert structure.atom_sites['A'].wyckoff_letter.value == 'e' + + def test_explicit_letter_preserved_on_first_update(self): + # (0.5,0,0) lies on both 'e' = (x,0,0) and the more special + # 'd' = (1/2,0,0); an explicit 'e' must be kept, not detected 'd'. + structure = self._structure() + structure.atom_sites.create( + label='E', + type_symbol='O', + fract_x=0.5, + fract_y=0.0, + fract_z=0.0, + adp_iso=0.5, + wyckoff_letter='e', + ) + structure._update_categories() + atom = structure.atom_sites['E'] + assert atom.wyckoff_letter.value == 'e' + assert atom.multiplicity.value == 6 + + def test_invalid_explicit_letter_raises_on_update(self): + import pytest + + structure = self._structure() + structure.atom_sites.create(label='Z', type_symbol='O', adp_iso=0.5, wyckoff_letter='z') + with pytest.raises(ValueError, match='Invalid Wyckoff letter'): + structure._update_categories() + + def test_same_letter_edit_snaps_off_orbit_coordinate(self): + # 'e' = (x,0,0): a small off-orbit nudge in fract_y (within the + # detection tolerance) keeps the letter 'e' and snaps fract_y back + # to 0 while fract_x stays free. + structure = self._structure() + structure.atom_sites.create( + label='E', + type_symbol='O', + fract_x=0.3, + fract_y=0.0, + fract_z=0.0, + adp_iso=0.5, + ) + structure._update_categories() + assert structure.atom_sites['E'].wyckoff_letter.value == 'e' + structure.atom_sites['E'].fract_x.value = 0.4 + structure.atom_sites['E'].fract_y.value = 0.0005 + structure._update_categories() + atom = structure.atom_sites['E'] + assert atom.wyckoff_letter.value == 'e' + assert abs(atom.fract_x.value - 0.4) < 1e-6 + assert abs(atom.fract_y.value) < 1e-6 + + def test_space_group_change_redetects(self): + structure = self._structure() + structure.atom_sites.create(label='A', type_symbol='O', adp_iso=0.5) + structure._update_categories() + assert structure.atom_sites['A'].multiplicity.value == 1 # Pm-3m 'a' + structure.space_group.name_h_m = 'F m -3 m' + structure._update_categories() + atom = structure.atom_sites['A'] + assert atom.wyckoff_letter.value == 'a' + assert atom.multiplicity.value == 4 # Fm-3m 'a' + + def test_minimizer_path_keeps_letter_fixed(self): + structure = self._structure() + structure.atom_sites.create( + label='E', + type_symbol='O', + fract_x=0.3, + fract_y=0.0, + fract_z=0.0, + adp_iso=0.5, + ) + structure._update_categories() + # A minimizer step varies the free axis; the letter stays fixed + # (no re-detection) and the free coordinate is preserved. + structure.atom_sites['E'].fract_x.value = 0.4 + structure._update_categories(called_by_minimizer=True) + assert structure.atom_sites['E'].wyckoff_letter.value == 'e' + assert abs(structure.atom_sites['E'].fract_x.value - 0.4) < 1e-6 + + def test_untabulated_group_preserves_letter_without_multiplicity(self, monkeypatch): + from easydiffraction.crystallography import crystallography as ecr + + monkeypatch.setattr(ecr, 'space_group_wyckoff_table', lambda *a, **k: None) + structure = self._structure() + structure.atom_sites.create(label='X', type_symbol='O', adp_iso=0.5, wyckoff_letter='a') + structure._update_categories() + atom = structure.atom_sites['X'] + assert atom.wyckoff_letter.value == 'a' + assert atom.multiplicity.value is None + + def test_no_record_contract_clears_multiplicity(self, monkeypatch): + from easydiffraction.crystallography import crystallography as ecr + + monkeypatch.setattr(ecr, 'space_group_wyckoff_table', lambda *a, **k: None) + structure = self._structure() + structure.atom_sites.create(label='X', type_symbol='O', adp_iso=0.5) + structure._update_categories() + assert structure.atom_sites['X'].multiplicity.value is None + assert '_atom_site.site_symmetry_multiplicity' in structure.as_cif + + def test_cif_round_trip_redrives_letter(self): + from easydiffraction.datablocks.structure.item.factory import StructureFactory + + structure = self._structure() + structure.atom_sites.create(label='A', type_symbol='O', adp_iso=0.5) + structure._update_categories() + reloaded = StructureFactory.from_cif_str(structure.as_cif) + reloaded._update_categories() + assert reloaded.atom_sites['A'].wyckoff_letter.value == 'a' + assert reloaded.atom_sites['A'].multiplicity.value == 1 diff --git a/tests/unit/easydiffraction/datablocks/structure/categories/test_space_group_wyckoff.py b/tests/unit/easydiffraction/datablocks/structure/categories/test_space_group_wyckoff.py new file mode 100644 index 000000000..16716343c --- /dev/null +++ b/tests/unit/easydiffraction/datablocks/structure/categories/test_space_group_wyckoff.py @@ -0,0 +1,136 @@ +# SPDX-FileCopyrightText: 2026 EasyScience contributors +# SPDX-License-Identifier: BSD-3-Clause +"""Tests for the derived space_group_wyckoff category (default + factory).""" + +import pytest + +from easydiffraction.crystallography import crystallography as ecr +from easydiffraction.datablocks.structure.item.base import Structure + + +def _cubic_structure(): + """Return a structure on the tabulated cubic group P m -3 m.""" + structure = Structure(name='s') + structure.space_group.name_h_m = 'P m -3 m' + structure._update_categories() + return structure + + +class TestSpaceGroupWyckoffFactory: + def test_supported_tags(self): + from easydiffraction.datablocks.structure.categories.space_group_wyckoff.factory import ( + SpaceGroupWyckoffFactory, + ) + + assert 'default' in SpaceGroupWyckoffFactory.supported_tags() + + def test_create_default(self): + from easydiffraction.datablocks.structure.categories.space_group_wyckoff.default import ( + SpaceGroupWyckoffCollection, + ) + from easydiffraction.datablocks.structure.categories.space_group_wyckoff.factory import ( + SpaceGroupWyckoffFactory, + ) + + obj = SpaceGroupWyckoffFactory.create('default') + assert isinstance(obj, SpaceGroupWyckoffCollection) + + +class TestDerivedRows: + def test_auto_populates_from_space_group(self): + structure = _cubic_structure() + table = ecr.space_group_wyckoff_table('P m -3 m', '1') + assert len(structure.space_group_wyckoff) == len(table) + for row in structure.space_group_wyckoff: + entry = table[row.letter.value] + assert row.multiplicity.value == entry['multiplicity'] + assert row.site_symmetry.value == str(entry['site_symmetry']) + + def test_uses_id_keys_of_multiplicity_plus_letter(self): + structure = _cubic_structure() + row = structure.space_group_wyckoff['1a'] + assert row.id.value == '1a' + assert row.letter.value == 'a' + assert row.multiplicity.value == 1 + + def test_preserves_site_symmetry_verbatim(self): + # Site-symmetry strings (including any dots) are stored exactly + # as the bundled table provides them, not normalised. + structure = _cubic_structure() + table = ecr.space_group_wyckoff_table('P m -3 m', '1') + for row in structure.space_group_wyckoff: + assert row.site_symmetry.value == str(table[row.letter.value]['site_symmetry']) + + def test_rebuilds_on_space_group_change(self): + structure = _cubic_structure() + assert '1a' in {r.id.value for r in structure.space_group_wyckoff} + structure.space_group.name_h_m = 'F m -3 m' + structure._update_categories() + ids = {r.id.value for r in structure.space_group_wyckoff} + # Fm-3m has no multiplicity-1 'a'; the stale Pm-3m key is gone. + assert '1a' not in ids + assert '4a' in ids + with pytest.raises(KeyError): + _ = structure.space_group_wyckoff['1a'] + + def test_empty_for_absent_group(self, monkeypatch): + structure = _cubic_structure() + assert len(structure.space_group_wyckoff) > 0 + monkeypatch.setattr(ecr, 'space_group_wyckoff_table', lambda *a, **k: None) + structure.space_group_wyckoff._replace_from_space_group() + assert len(structure.space_group_wyckoff) == 0 + + +class TestReadOnly: + def test_rejects_all_public_mutation(self): + structure = _cubic_structure() + wy = structure.space_group_wyckoff + row = wy['1a'] + with pytest.raises(ValueError, match='read-only'): + wy.add(row) + with pytest.raises(ValueError, match='read-only'): + wy.create() + with pytest.raises(ValueError, match='read-only'): + wy.remove('1a') + with pytest.raises(ValueError, match='read-only'): + wy['1a'] = row + with pytest.raises(ValueError, match='read-only'): + del wy['1a'] + + def test_from_cif_ignores_incoming_loop(self): + # A hand-edited _space_group_Wyckoff loop must be discarded; the + # table is derived from the space group, not read from CIF. + from easydiffraction.datablocks.structure.item.factory import StructureFactory + + cif = ( + 'data_x\n' + "_space_group.name_H-M_alt 'P m -3 m'\n" + '_space_group.IT_coordinate_system_code 1\n' + 'loop_\n' + '_space_group_Wyckoff.id\n' + '_space_group_Wyckoff.letter\n' + '_space_group_Wyckoff.multiplicity\n' + '_space_group_Wyckoff.site_symmetry\n' + '_space_group_Wyckoff.coords_xyz\n' + ' BOGUS bogus 999 zzz (9,9,9)\n' + ) + structure = StructureFactory.from_cif_str(cif) + assert all(r.id.value != 'BOGUS' for r in structure.space_group_wyckoff) + + +class TestSerialization: + def test_omitted_from_project_cif(self): + structure = _cubic_structure() + assert '_space_group_Wyckoff' not in structure.as_cif + + def test_appears_in_report_output(self): + from easydiffraction.io.cif import iucr_writer + + structure = _cubic_structure() + lines: list[str] = [] + iucr_writer._write_space_group_wyckoff_section(lines, structure) + body = '\n'.join(lines) + assert '_space_group_Wyckoff.id' in body + assert '_space_group_Wyckoff.coords_xyz' in body + # Representative coordinate only (compact), never the full orbit. + assert max(len(line) for line in lines) < 80 diff --git a/tests/unit/easydiffraction/display/plotters/test_plotly.py b/tests/unit/easydiffraction/display/plotters/test_plotly.py index 51245c04c..53200f98a 100644 --- a/tests/unit/easydiffraction/display/plotters/test_plotly.py +++ b/tests/unit/easydiffraction/display/plotters/test_plotly.py @@ -885,7 +885,7 @@ def fake_show_figure(self, fig): x=np.array([1.0, 2.0, 3.0]), y_meas=np.array([10.0, 12.0, 11.0]), y_calc=np.array([9.0, 11.0, 10.5]), - y_resid=None, + y_resid=np.array([1.0, 1.0, 0.5]), bragg_tick_sets=( BraggTickSet( phase_id='phase-a', @@ -905,9 +905,74 @@ def fake_show_figure(self, fig): ), ) + # A full main + Bragg + residual composite renders at exactly the + # explicit height; reduced layouts derive a smaller height instead. assert captured['fig'].layout.height == 800 +def test_plot_powder_meas_vs_calc_keeps_top_and_bottom_rows_fixed(monkeypatch): + """Top and residual rows keep a fixed pixel height.""" + import easydiffraction.display.plotters.plotly as pp + + from easydiffraction.display.plotters.base import BraggTickSet + from easydiffraction.display.plotters.base import PowderMeasVsCalcSpec + + captured = {} + + def fake_show_figure(self, fig): + captured.setdefault('figures', []).append(fig) + + monkeypatch.setattr(pp.PlotlyPlotter, '_show_figure', fake_show_figure) + + bragg_tick_sets = ( + BraggTickSet( + phase_id='phase-a', + x=np.array([1.5]), + h=np.array([1]), + k=np.array([0]), + ell=np.array([1]), + f_squared_calc=np.array([100.0]), + f_calc=np.array([10.0]), + ), + ) + + def plot_spec(*, with_bragg: bool, with_residual: bool) -> PowderMeasVsCalcSpec: + return PowderMeasVsCalcSpec( + x=np.array([1.0, 2.0, 3.0]), + y_meas=np.array([10.0, 12.0, 11.0]), + y_calc=np.array([9.0, 11.0, 10.5]), + y_resid=np.array([1.0, 1.0, 0.5]) if with_residual else None, + bragg_tick_sets=bragg_tick_sets if with_bragg else (), + axes_labels=['2θ (deg)', 'Intensity (arb. units)'], + title='Powder', + residual_height_fraction=0.25, + bragg_peaks_height_fraction=0.10, + height=None, + ) + + plotter = pp.PlotlyPlotter() + plotter.plot_powder_meas_vs_calc(plot_spec=plot_spec(with_bragg=True, with_residual=True)) + plotter.plot_powder_meas_vs_calc(plot_spec=plot_spec(with_bragg=True, with_residual=False)) + plotter.plot_powder_meas_vs_calc(plot_spec=plot_spec(with_bragg=False, with_residual=True)) + plotter.plot_powder_meas_vs_calc(plot_spec=plot_spec(with_bragg=False, with_residual=False)) + + full, main_bragg, main_resid, main_only = captured['figures'] + + def row_pixels(fig, axis_name: str) -> float: + axis = getattr(fig.layout, axis_name) + plot_area_height = fig.layout.height - fig.layout.margin.t - fig.layout.margin.b + return plot_area_height * (axis.domain[1] - axis.domain[0]) + + # The top (main) row keeps the same pixel height in every layout. + assert row_pixels(main_bragg, 'yaxis') == pytest.approx(row_pixels(full, 'yaxis')) + assert row_pixels(main_resid, 'yaxis') == pytest.approx(row_pixels(full, 'yaxis')) + assert row_pixels(main_only, 'yaxis') == pytest.approx(row_pixels(full, 'yaxis')) + # The residual row keeps its height whether or not Bragg shows. + assert row_pixels(main_resid, 'yaxis2') == pytest.approx(row_pixels(full, 'yaxis3')) + # Hiding rows shrinks the figure instead of stretching the top row. + assert main_only.layout.height < main_resid.layout.height < full.layout.height + + def test_plot_powder_meas_vs_calc_skips_bragg_row_when_no_ticks(monkeypatch): import easydiffraction.display.plotters.plotly as pp diff --git a/tests/unit/easydiffraction/report/test_fit_plot.py b/tests/unit/easydiffraction/report/test_fit_plot.py index 0a231f1a8..898d825bf 100644 --- a/tests/unit/easydiffraction/report/test_fit_plot.py +++ b/tests/unit/easydiffraction/report/test_fit_plot.py @@ -72,3 +72,43 @@ def test_fit_plot_axis_styles_expose_shared_diagonal_color(): styles = fit_plot_axis_styles() assert styles['diag_rgb'] == '190,199,208' + + +def test_fit_plot_geometry_keeps_panel_heights_fixed_across_rows(): + from easydiffraction.report.fit_plot import _FIGURE_AXIS_HEIGHT_TO_WIDTH + from easydiffraction.report.fit_plot import _FIGURE_AXIS_WIDTH_CM + from easydiffraction.report.fit_plot import fit_plot_geometry + + def fit_data(*, phases: int, residual: bool) -> dict: + data = {'bragg_tick_sets': ['phase'] * phases} + data['series'] = {'diff': [0.0]} if residual else {} + return data + + def stack_height_cm(geometry: dict, row_count: int) -> float: + return ( + geometry['main_height_cm'] + + geometry['bragg_height_cm'] + + geometry['residual_height_cm'] + + (row_count - 1) * geometry['vertical_sep_cm'] + ) + + full = fit_plot_geometry(fit_data(phases=1, residual=True)) + main_bragg = fit_plot_geometry(fit_data(phases=1, residual=False)) + main_resid = fit_plot_geometry(fit_data(phases=0, residual=True)) + main_only = fit_plot_geometry(fit_data(phases=0, residual=False)) + two_phase = fit_plot_geometry(fit_data(phases=2, residual=True)) + + # The main panel keeps the same cm height in every layout. + assert main_bragg['main_height_cm'] == pytest.approx(full['main_height_cm']) + assert main_resid['main_height_cm'] == pytest.approx(full['main_height_cm']) + assert main_only['main_height_cm'] == pytest.approx(full['main_height_cm']) + # The residual panel keeps its height with or without Bragg ticks. + assert main_resid['residual_height_cm'] == pytest.approx(full['residual_height_cm']) + # The Bragg panel grows linearly with the phase count. + assert two_phase['bragg_height_cm'] == pytest.approx(2 * full['bragg_height_cm']) + # The reference three-row figure keeps its nominal height; reduced + # layouts are shorter and extra phases make it taller. + reference_cm = _FIGURE_AXIS_WIDTH_CM * _FIGURE_AXIS_HEIGHT_TO_WIDTH + assert stack_height_cm(full, 3) == pytest.approx(reference_cm) + assert stack_height_cm(main_only, 1) < reference_cm + assert stack_height_cm(two_phase, 3) > reference_cm diff --git a/tools/check_packaged_db.py b/tools/check_packaged_db.py index d0bac73bd..efb844ecf 100644 --- a/tools/check_packaged_db.py +++ b/tools/check_packaged_db.py @@ -6,8 +6,10 @@ of the project's full dependency tree: it opens the wheel, reads ``space_groups.json.gz`` straight from it, and asserts the data is shipped as package data, that the obsolete ``space_groups.pkl.gz`` is gone, and that the -archive covers all 230 IT groups plus the cryspy coordinate-code alias surface. -Exits non-zero on any problem so a packaging regression fails the caller. +archive covers all 230 IT groups plus the cryspy coordinate-code alias +surface, and that no Wyckoff ``coords_xyz`` template is in cctbx operator form +(canonical ITA form only). Exits non-zero on any problem so a packaging +regression fails the caller. Usage: ``python tools/check_packaged_db.py [path/to/wheel]`` (defaults to the newest wheel in ``dist/``). @@ -17,6 +19,7 @@ import gzip import json +import re import sys import zipfile from pathlib import Path @@ -26,6 +29,9 @@ _REQUIRED_KEYS = [(14, '-b1'), (3, '-a1'), (1, None)] # Accepted seed record count (see the space-group-database ADR provenance). _EXPECTED_RECORD_COUNT = 816 +# Wyckoff coords_xyz must be canonical ITA form, never cctbx operator form +# (e.g. ``1/2*x-1/2*y``), which breaks symmetry-constraint detection. +_OPERATOR_FORM = re.compile(r'[0-9.]\s*\*\s*[xyz]|[xyz]\s*\*') def _wheel_path(argv: list[str]) -> Path: @@ -59,6 +65,19 @@ def main(argv: list[str]) -> None: if len(keys) != _EXPECTED_RECORD_COUNT: sys.exit(f'packaged database has {len(keys)} unique keys, expected {_EXPECTED_RECORD_COUNT}') + operator_form = [ + (record['IT_number'], record['IT_coordinate_system_code'], letter, template) + for record in records + for letter, position in record['Wyckoff_positions'].items() + for template in position['coords_xyz'] + if _OPERATOR_FORM.search(template) + ] + if operator_form: + sys.exit( + f'packaged database has {len(operator_form)} operator-form coords_xyz ' + f'(canonical ITA form required); first: {operator_form[0]}' + ) + print( f'packaged DB OK in {wheel.name}: ' f'{len({key[0] for key in keys})} IT groups, {len(records)} settings'