From 6c294971f1b3a28cb10535edda755f0ced67bf20 Mon Sep 17 00:00:00 2001 From: acorrao <95718914+AdamCorrao@users.noreply.github.com> Date: Wed, 7 Oct 2026 09:46:11 -0400 Subject: [PATCH 01/14] fix(symmetry): monoclinic unique axis from the 2-fold axis or the mirror normal (A102) `_monoclinic_unique_axis` looked only for a proper 2-fold, so `cell_constraints` raised "could not determine the monoclinic unique axis" for every setting of the mirror-only groups Pm, Pc, Cm and Cc (space groups 6-9, 41 gemmi settings) since v0.1.1. The unique axis is now the invariant direction of det(R)*R for any operation with |trace| = 1: the 2-fold axis or the mirror/glide normal. One shared helper for every cell rule (the re/03b cell tie groups, re/04's coarse groups, re/05/re/06). Deliberate change (devkit tasks/deliberate-changes.md #4): the legacy TOPAS writer and the easydiffraction policy/builder no longer refuse these groups early. Tests: Pm/Pc/Cm/Cc and P2, P2_1/c, C2/c in a-, b- and c-unique settings and other cell choices, plus every monoclinic setting in gemmi's table (115). Assisted-by: claude-code:claude-opus-5-5 --- src/powderline/symmetry.py | 15 ++++++++------ tests/test_topas_symmetry.py | 40 ++++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 6 deletions(-) diff --git a/src/powderline/symmetry.py b/src/powderline/symmetry.py index 90d5387..eec798b 100644 --- a/src/powderline/symmetry.py +++ b/src/powderline/symmetry.py @@ -158,20 +158,23 @@ def _expanded_ops(sg: gemmi.SpaceGroup) -> list[tuple[np.ndarray, np.ndarray]]: def _monoclinic_unique_axis(ops: list[tuple[np.ndarray, np.ndarray]]) -> str: - """Unique axis of a monoclinic group from its proper 2-fold direction. + """Unique axis of a monoclinic group: its 2-fold axis or its mirror/glide normal. - The single proper 2-fold (det +1, trace -1) fixes the unique axis; its - invariant direction maps to the dominant cell axis a/b/c. + Every operation of a monoclinic point group other than 1 and -1 is a proper + 2-fold (trace -1) or a mirror (trace +1). For either, ``det(R) R`` is a proper + 2-fold about the unique axis, whose invariant direction maps to the dominant + cell axis a/b/c. Mirror-only groups (Pm, Pc, Cm, Cc) have no proper 2-fold, + so the mirror normal is needed (A102). """ for R, _t in ops: - if abs(np.linalg.det(R) - 1.0) < _MAT_TOL and abs(np.trace(R) + 1.0) < _MAT_TOL: - evals, evecs = np.linalg.eig(R) + if abs(abs(np.trace(R)) - 1.0) < _MAT_TOL: + evals, evecs = np.linalg.eig(np.sign(np.linalg.det(R)) * R) for i in range(3): if abs(evals[i].real - 1.0) < _MAT_TOL and abs(evals[i].imag) < _MAT_TOL: direction = np.abs(evecs[:, i].real) return _LENGTHS[int(np.argmax(direction))] raise SymmetryError( - "could not determine the monoclinic unique axis (no proper 2-fold found)" + "could not determine the monoclinic unique axis (no 2-fold axis or mirror found)" ) diff --git a/tests/test_topas_symmetry.py b/tests/test_topas_symmetry.py index 71deb03..8b71a24 100644 --- a/tests/test_topas_symmetry.py +++ b/tests/test_topas_symmetry.py @@ -45,6 +45,46 @@ def test_monoclinic_unique_a_and_c(): assert sym.cell_constraints("P 1 1 2/m").free_angles == ("gamma",) +# Mirror-only groups (Pm, Pc, Cm, Cc: no proper 2-fold, the axis is the mirror +# normal) and 2-fold groups, in all three unique-axis settings and several cell +# choices (A102: these four groups raised "could not determine the monoclinic +# unique axis" before). +@pytest.mark.parametrize("space_group, axis", [ + ("P 1 m 1", "b"), ("P 1 1 m", "c"), ("P m 1 1", "a"), + ("P 1 c 1", "b"), ("P 1 n 1", "b"), ("P 1 1 a", "c"), ("P 1 1 n", "c"), + ("P b 1 1", "a"), ("P c 1 1", "a"), + ("C 1 m 1", "b"), ("A 1 1 m", "c"), ("B m 1 1", "a"), ("I 1 m 1", "b"), + ("C 1 c 1", "b"), ("I 1 a 1", "b"), ("A 1 1 a", "c"), ("I 1 1 b", "c"), + ("B b 1 1", "a"), ("C c 1 1", "a"), + ("P 1 2 1", "b"), ("P 1 1 2", "c"), ("P 2 1 1", "a"), + ("P 1 21/c 1", "b"), ("P 1 1 21/a", "c"), ("P 21/b 1 1", "a"), + ("C 1 2/c 1", "b"), ("I 1 2/a 1", "b"), ("A 1 1 2/a", "c"), ("B 2/b 1 1", "a"), +]) +def test_monoclinic_unique_axis_every_setting(space_group, axis): + r = sym.cell_constraints(space_group) + free_angle = {"a": "alpha", "b": "beta", "c": "gamma"}[axis] + assert r.crystal_system == "monoclinic" + assert r.unique_axis == axis + assert r.length_groups == (("a",), ("b",), ("c",)) + assert r.free_angles == (free_angle,) + assert r.fixed_angles == tuple(a for a in ("alpha", "beta", "gamma") if a != free_angle) + + +def test_monoclinic_unique_axis_all_gemmi_settings(): + """Every monoclinic setting in gemmi's table: the axis is the H-M symbol's non-'1' slot.""" + import gemmi + + checked = 0 + for sg in gemmi.spacegroup_table(): + if sg.crystal_system_str() != "monoclinic": + continue + slots = sg.hm.split()[1:] + expected = "abc"[[i for i, s in enumerate(slots) if s != "1"][0]] + assert sym.cell_constraints(sg.xhm()).unique_axis == expected, sg.xhm() + checked += 1 + assert checked == 115 + + def test_tetragonal_hexagonal_ortho_triclinic(): assert sym.cell_constraints("P 4/m m m").length_groups == (("a", "b"), ("c",)) hexr = sym.cell_constraints("P 6/m m m") From a0c1a1587ecff7fc3be4541e5ed846f783831f7d Mon Sep 17 00:00:00 2001 From: acorrao <95718914+AdamCorrao@users.noreply.github.com> Date: Wed, 7 Oct 2026 11:15:02 -0400 Subject: [PATCH 02/14] feat(core): one phase block, Phase[P] with refinable values and canonical space groups (A93-A96, A100, A105) The core phase model is now one block: `Phase[P]`, `UnitCell[P]` and `Atom[P]`, generic over the parameter type P (RefinableParameter, or BoundedRefinableParameter for engines that honor bounds; one P for cell and atoms, review N6). Every structural quantity (cell a..gamma; atom x, y, z, occupancy, Uiso, U11..U23) is `[value, refine_flag]`; element, ADP, a stated Multiplicity and the space group stay plain values. Engine schemas subclass `Phase[TheirType]` and add their own fields flat (gsasii: scale, peak_broadening). - Value rules unchanged in substance, now on `.value` (A63, A69, A73, A76-A78); canonicalization replaces `.value` and keeps the flag and bounds. - No null and no defaults (A96): occupancy is required; Multiplicity, Uiso and Uaniso may be left out but never null; a dump leaves them out, so it validates again. - No phase_name: the phases dict key is the name (A100). - `space_group` must be gemmi's canonical xhm() name exactly (A105); any other spelling is an error naming the canonical form (both settings / origins for a bare rhombohedral / two-origin symbol). New engine-free `symmetry.canonical_space_group`; the lenient resolve_space_group stays for the legacy paths. - Reserved names: an engine subclass that redeclares space_group, unit_cell or atoms raises TypeError at class definition. - `CoreModel` base + `CoreJsonSchema`: readable $defs names (`Atom`, not `Atom_RefinableParameter_`); qualified fallback if two parameter types ever share a document (devkit tasks/re03b-spike-generics.md). - Removed: PhaseStructure, StructureRefinableParameter, StructureBoundedRefinableParameter and the null-value schema note (A71's structure part is superseded by A93/A96). Flag and bound rules (F1-F4) follow in a later commit. Nothing at runtime consumes the phase model yet; the legacy 0.26.0 schema is unchanged. Tests: test_schema_core reworked by ID (17 Structure*-only tests and 14 Structure* numeric parametrizations removed, 22 test_phase_structure_* renamed test_phase_*), plus new phase-block tests. Assisted-by: claude-code:claude-opus-5-5 Assisted-by: claude-code:claude-sonnet-5-5 --- src/powderline/schema_core.py | 309 ++++++++------ src/powderline/symmetry.py | 38 ++ tests/test_schema_core.py | 730 ++++++++++++++++++++-------------- 3 files changed, 650 insertions(+), 427 deletions(-) diff --git a/src/powderline/schema_core.py b/src/powderline/schema_core.py index 89997f2..12a8ac7 100644 --- a/src/powderline/schema_core.py +++ b/src/powderline/schema_core.py @@ -21,9 +21,11 @@ from __future__ import annotations +import inspect import json import math -from typing import Annotated, Any, Literal, Optional +import re +from typing import Annotated, Any, Generic, Literal, Optional, TypeVar import gemmi import numpy as np @@ -42,10 +44,11 @@ model_serializer, model_validator, ) +from pydantic.json_schema import DEFAULT_REF_TEMPLATE, GenerateJsonSchema, JsonSchemaMode from pydantic_core import InitErrorDetails, PydanticCustomError from powderline.exceptions import StructuredWarning, SymmetryError -from powderline.symmetry import analyze_site, check_cell, check_uij, resolve_space_group +from powderline.symmetry import analyze_site, canonical_space_group, check_cell, check_uij # --- versions (A40, A65) --------------------------------------------------- @@ -112,6 +115,40 @@ def _array_schema(title: str, items: list[dict], description: str, **extra) -> d _STRICT = ConfigDict(extra="forbid") +class CoreJsonSchema(GenerateJsonSchema): + """JSON Schema generator that names a parametrized core model after its origin. + + pydantic names ``Atom[RefinableParameter]`` ``Atom_RefinableParameter_`` in + ``$defs``; a recipe document holds one engine's schema (one parameter type), + so plain ``Atom`` is unambiguous and readable. Only the short candidate names + (no module path, no id) are shortened: pydantic's content-based dedup then + falls back to its qualified names if two parametrizations ever share one + document, and the id-bearing name it needs unique is left intact (devkit + ``tasks/re03b-spike-generics.md``). + """ + + def normalize_name(self, name: str) -> str: + if "." not in name and ":" not in name: + name = re.sub(r"\[[^\]]*\]", "", name) + return super().normalize_name(name) + + +class CoreModel(BaseModel): + """Base of the core models: ``extra='forbid'`` and :class:`CoreJsonSchema` by default. + + Engine schemas build on it (directly, or through :class:`CoreRecipe` and + :class:`Phase`), so their generated JSON Schema is readable too. + """ + + model_config = _STRICT + + @classmethod + def model_json_schema(cls, by_alias: bool = True, ref_template: str = DEFAULT_REF_TEMPLATE, + schema_generator: type[GenerateJsonSchema] = CoreJsonSchema, + mode: JsonSchemaMode = "validation", **kwargs) -> dict[str, Any]: + return super().model_json_schema(by_alias, ref_template, schema_generator, mode, **kwargs) + + # --- numbers ----------------------------------------------------------------- @@ -131,7 +168,7 @@ def _integer_not_text_or_bool(v: Any) -> Any: CoreInt = Annotated[int, BeforeValidator(_integer_not_text_or_bool)] -# --- parameter models (A20, A32, A71) --------------------------------------- +# --- parameter models (A20, A32, A93) --------------------------------------- def _finite(name: str, v: Optional[float]) -> Optional[float]: @@ -145,8 +182,7 @@ class RefinableParameter(BaseModel): Used where the engine has no native bounds for the quantity. A 4-element list here is an error, never silently truncated (A20). ``value`` is - required; see :class:`StructureRefinableParameter` for the quantities whose - starting value may come from the phase structure. + required (never ``null``, A93/A96). """ model_config = _STRICT @@ -184,9 +220,7 @@ def _to_list(self) -> list: @classmethod def __get_pydantic_json_schema__(cls, core_schema, handler) -> dict: - nullable = cls.model_fields["value"].annotation is not float - return _array_schema(cls.__name__, [_NUMBER_OR_NULL if nullable else _NUMBER, _BOOLEAN], - "[value, refine_flag]" + (_NULL_VALUE_NOTE if nullable else "")) + return _array_schema(cls.__name__, [_NUMBER, _BOOLEAN], "[value, refine_flag]") class BoundedRefinableParameter(BaseModel): @@ -194,8 +228,8 @@ class BoundedRefinableParameter(BaseModel): ``min``/``max`` may each be ``null`` (unbounded on that side). When given, ``min <= max`` and ``min <= value <= max``. Used only where the engine - honors the bounds natively (A20, A33). ``value`` is required; see - :class:`StructureBoundedRefinableParameter`. + honors the bounds natively (A20, A33). ``value`` is required (never + ``null``, A93/A96). """ model_config = _STRICT @@ -229,8 +263,7 @@ def _bounds(self) -> "BoundedRefinableParameter": lo, hi = self.min, self.max if lo is not None and hi is not None and lo > hi: raise ValueError(f"min ({lo}) must be <= max ({hi})") - if self.value is not None: - check_within_bounds(self.value, self, "value") + check_within_bounds(self.value, self, "value") return self @model_serializer @@ -239,44 +272,15 @@ def _to_list(self) -> list: @classmethod def __get_pydantic_json_schema__(cls, core_schema, handler) -> dict: - nullable = cls.model_fields["value"].annotation is not float - return _array_schema(cls.__name__, - [_NUMBER_OR_NULL if nullable else _NUMBER, _BOOLEAN, - _NUMBER_OR_NULL, _NUMBER_OR_NULL], - "[value, refine_flag, min, max]; min/max may be null" - + (_NULL_VALUE_NOTE if nullable else "")) - - -_NULL_VALUE_NOTE = "; value null = start from the phase structure's value" - - -class StructureRefinableParameter(RefinableParameter): - """:class:`RefinableParameter` for a quantity that mirrors a phase-structure value. - - For cell lengths/angles and atom x/y/z, occupancy, Uiso/Uaniso only (A71): - ``value`` may be ``null``, meaning "start from the structure's value" - (today's behavior: the GSAS-II setters skip a null value). - """ - - value: Optional[CoreFloat] - - -class StructureBoundedRefinableParameter(BoundedRefinableParameter): - """:class:`BoundedRefinableParameter` for a quantity that mirrors a phase-structure value. - - ``value`` may be ``null`` (start from the structure's value, A71); the - bounds are then checked against that value by the phase-level validator - (:func:`check_within_bounds`). - """ - - value: Optional[CoreFloat] + return _array_schema(cls.__name__, [_NUMBER, _BOOLEAN, _NUMBER_OR_NULL, _NUMBER_OR_NULL], + "[value, refine_flag, min, max]; min/max may be null") def check_within_bounds(value: float, param: BoundedRefinableParameter, field: str) -> None: """Raise ``ValueError`` unless ``param.min <= value <= param.max`` (open ends pass). - Also used at the phase level for a null-valued - :class:`StructureBoundedRefinableParameter`, with the structure's value. + Also used at the phase level, on a value canonicalized after the + parameter's own check (review N4). """ if param.min is not None and value < param.min: raise ValueError(f"{field} ({value}) is below min ({param.min})") @@ -497,84 +501,117 @@ def _coefficients_finite(cls, v): return v -# --- phase structure (A32, A35, A62, A63, A69, A73) ------------------------- +# --- phase block (A93-A96, A100, A105; values: A63, A69, A73, A76-A78) ------- +#: The parameter type of a phase block: :class:`RefinableParameter`, or +#: :class:`BoundedRefinableParameter` for an engine that honors bounds (A20). +#: One type for every structural field, cell and atoms (review N6). +P = TypeVar("P", RefinableParameter, BoundedRefinableParameter) -class UnitCell(BaseModel): - """Cell constants (values, not refinables). No ``volume``: every engine - derives it from a..gamma (A75).""" - model_config = _STRICT +def _absent_not_null(v: Any, info) -> Any: + """A field that may be left out is never ``null`` (A96): reject an explicit null.""" + if v is None: + raise ValueError(f"{info.field_name} must not be null; leave it out instead") + return v - a: CoreFloat = Field(json_schema_extra=_unit(UNIT_ANGSTROM)) - b: CoreFloat = Field(json_schema_extra=_unit(UNIT_ANGSTROM)) - c: CoreFloat = Field(json_schema_extra=_unit(UNIT_ANGSTROM)) - alpha: CoreFloat = Field(json_schema_extra=_unit(UNIT_DEGREE)) - beta: CoreFloat = Field(json_schema_extra=_unit(UNIT_DEGREE)) - gamma: CoreFloat = Field(json_schema_extra=_unit(UNIT_DEGREE)) + +def _absent_not_null_schema(unit: Optional[str] = None): + """JSON Schema of a field that may be left out but is never null (A96). + + The field is ``Optional`` only so that "left out" can be held as ``None``; + the schema shows the non-null type, no ``"default": null``, and the unit. + """ + def extra(schema: dict) -> None: + schema.pop("default", None) + any_of = schema.pop("anyOf", None) + if any_of: + schema.update(next(s for s in any_of if s.get("type") != "null")) + if unit: + schema.update(_unit(unit)) + return extra + + +class UnitCell(CoreModel, Generic[P]): + """Cell constants, each a refinable parameter. No ``volume``: every engine + derives it from a..gamma (A75). The values must fit the space group (A77, + checked by :class:`Phase`).""" + + a: P = Field(json_schema_extra=_unit(UNIT_ANGSTROM)) + b: P = Field(json_schema_extra=_unit(UNIT_ANGSTROM)) + c: P = Field(json_schema_extra=_unit(UNIT_ANGSTROM)) + alpha: P = Field(json_schema_extra=_unit(UNIT_DEGREE)) + beta: P = Field(json_schema_extra=_unit(UNIT_DEGREE)) + gamma: P = Field(json_schema_extra=_unit(UNIT_DEGREE)) @field_validator("a", "b", "c") @classmethod def _length(cls, v, info): - if not (math.isfinite(v) and v > 0): - raise ValueError(f"cell length {info.field_name} must be positive, got {v}") + if not v.value > 0: + raise ValueError(f"cell length {info.field_name} must be positive, got {v.value}") return v @field_validator("alpha", "beta", "gamma") @classmethod def _angle(cls, v, info): - if not (math.isfinite(v) and 0 < v < 180): - raise ValueError(f"cell angle {info.field_name} must be between 0 and 180 degrees, got {v}") + if not 0 < v.value < 180: + raise ValueError(f"cell angle {info.field_name} must be between 0 and 180 degrees, got {v.value}") return v _UANISO_KEYS = ("U11", "U22", "U33", "U12", "U13", "U23") -class Atom(BaseModel): - """One atom of a phase structure (values, not refinables). +class Atom(CoreModel, Generic[P]): + """One atom of a phase: coordinates, occupancy and ADPs are refinable parameters. Coordinates are fractional JSON numbers. A special position is declared by - stating it to at least 6 decimals. The validated phase holds canonical - (exact) coordinates and reports every adjustment (A73). ``Multiplicity`` is - optional and, when stated, must match the derived value (A69 T3). - ``occupancy`` is 0 to 1 inclusive, even where an engine would accept more - (A76). ``Uaniso`` must respect the site symmetry: within 1e-6 A^2 it is set - to the symmetric values and reported, beyond that it is an error (A78). + stating it to at least 6 decimals; the validated phase holds the canonical + (exact) values and reports every adjustment (A73). ``occupancy`` is required + and 0 to 1 inclusive (A76, A96). ``Multiplicity`` is optional and, when + stated, must match the derived value (A69 T3). ``ADP`` selects ``Uiso`` or + ``Uaniso`` (all six of U11..U23); the other is left out. ``Uaniso`` must + respect the site symmetry: within 1e-6 A^2 it is set to the symmetric values + and reported, beyond that it is an error (A78). Nothing is ``null`` (A96). """ - model_config = _STRICT - element: str = Field(description="Bare element symbol, exactly as in the periodic table (e.g. 'Fe')") - x: CoreFloat = Field(description="Fractional x coordinate") - y: CoreFloat = Field(description="Fractional y coordinate") - z: CoreFloat = Field(description="Fractional z coordinate") - occupancy: CoreFloat = Field(default=1.0, ge=0.0, le=1.0, description="Site occupancy, 0 to 1 inclusive (A76)") - Multiplicity: Optional[CoreInt] = Field(default=None, description="Site multiplicity; cross-checked when stated") + x: P = Field(description="Fractional x coordinate") + y: P = Field(description="Fractional y coordinate") + z: P = Field(description="Fractional z coordinate") + occupancy: P = Field(description="Site occupancy, 0 to 1 inclusive (A76)") + Multiplicity: Optional[CoreInt] = Field(default=None, json_schema_extra=_absent_not_null_schema(), + description="Site multiplicity; optional, cross-checked when stated") ADP: Literal["Uiso", "Uaniso"] = Field(description="Displacement-parameter type") - Uiso: Optional[CoreFloat] = Field(default=None, json_schema_extra=_unit(UNIT_ANGSTROM2)) - Uaniso: Optional[dict[Literal["U11", "U22", "U33", "U12", "U13", "U23"], CoreFloat]] = Field( - default=None, json_schema_extra=_unit(UNIT_ANGSTROM2), - description="All six of U11, U22, U33, U12, U13, U23") + Uiso: Optional[P] = Field(default=None, json_schema_extra=_absent_not_null_schema(UNIT_ANGSTROM2), + description="Isotropic ADP; given when ADP is 'Uiso'") + Uaniso: Optional[dict[Literal["U11", "U22", "U33", "U12", "U13", "U23"], P]] = Field( + default=None, json_schema_extra=_absent_not_null_schema(UNIT_ANGSTROM2), + description="All six of U11, U22, U33, U12, U13, U23; given when ADP is 'Uaniso'") + + _not_null = field_validator("Multiplicity", "Uiso", "Uaniso", mode="before")(_absent_not_null) @field_validator("element") @classmethod def _element(cls, v: str) -> str: return check_element_symbol(v) - @field_validator("x", "y", "z", "occupancy", "Uiso") - @classmethod - def _finite(cls, v, info): - return _finite(info.field_name, v) - - @field_validator("Uaniso") + @field_validator("occupancy") @classmethod - def _uaniso_finite(cls, v): - if v is not None: - for k, u in v.items(): - _finite(k, u) + def _occupancy(cls, v): + if not 0.0 <= v.value <= 1.0: + raise ValueError(f"occupancy must be between 0 and 1 inclusive, got {v.value}") return v + @model_serializer(mode="wrap") + def _omit_absent(self, handler): + """Leave out absent optional fields, so a dump validates again (no ``null``, A96).""" + data = handler(self) + for name in ("Multiplicity", "Uiso", "Uaniso"): + if getattr(self, name) is None: + data.pop(name, None) + return data + @model_validator(mode="after") def _adp_block(self) -> "Atom": if self.ADP == "Uiso": @@ -590,46 +627,67 @@ def _adp_block(self) -> "Atom": return self -class PhaseStructure(BaseModel): - """A phase's crystal structure: space group, cell, atoms (core-owned, A35). +#: Phase fields owned by core; an engine subclass may not redeclare them (A93). +RESERVED_PHASE_FIELDS = ("space_group", "unit_cell", "atoms") - Validation derives each atom's site (multiplicity, DOF) from the space group - (:func:`powderline.symmetry.analyze_site`). It rejects ambiguous positions - and stated multiplicities that don't match. It replaces stated coordinates - with the canonical ones, so every engine receives the same exact structure. - Each adjustment is reported by :meth:`warnings` (A73). The unit cell must - fit the space group exactly (A77), and anisotropic ADPs the site symmetry - (A78). - Every structural problem is reported at its own location (A70): - ``unit_cell``, ``atoms.