Skip to content

fix(#284) split kappa-arm tilt direction from equivalent-Eulerian chi axis#286

Merged
prjemian merged 1 commit into
mainfrom
284-kappa-eulerian-chi-axis
May 20, 2026
Merged

fix(#284) split kappa-arm tilt direction from equivalent-Eulerian chi axis#286
prjemian merged 1 commit into
mainfrom
284-kappa-eulerian-chi-axis

Conversation

@prjemian

Copy link
Copy Markdown
Collaborator

Summary

Issue #284 reported that kappa4cv bisecting and kappa6c bisecting_vertical lost solutions for sapphire asymmetric reflections — (0, 1, 2), (0, 0, 6), (1, 1, 3) all return zero solutions in v0.11.0.

Root cause

Issue #252 conflated two distinct concepts in the single YAML field kappa_chi_eq:

  1. Kappa-arm tilt direction — input to Walko's formula n_kappa = cos(α)*n_komega_unsigned + sin(α)*tilt_direction. For kappa4cv this is +vertical (kappa arm lies in the transverse-vertical plane, per Walko 2016 Fig. 3).
  2. Equivalent-Eulerian chi pseudo-angle axis — the axis the kappa→Eulerian decomposition rotates about for the virtual chi angle. For kappa4cv this must be +longitudinal to match fourcv's chi axis (the non-kappa Eulerian preset kappa4cv is mechanically equivalent to).

Setting both to the same value (+vertical) broke the second role: the kappa equivalent-Eulerian decomposition's reachable Bragg locus shrank away from the locus reachable by fourcv bisecting. Cross-checked against libhkl K4CV, which has chi-eq = +longitudinal and finds the missing solutions.

For kappa4ch the two roles coincide (both +longitudinal), so it was already correct.

Fix (Option C from investigation)

Decouple the two meanings without a breaking schema change:

  • New optional top-level YAML field kappa_eulerian_chi that sets n_chi_eq verbatim when present.
  • When absent, auto-derive n_chi_eq as the first basis direction perpendicular to n_komega in the conventional order (+longitudinal, +vertical, +transverse) — yields +longitudinal for every shipped kappa preset.
  • The existing kappa_chi_eq field continues to drive Walko's kappa-arm tilt formula in _resolve_axis (unchanged).

Source changes

  • geometry_loader.py: parse the new field, derive when absent, build KappaPseudoAngleConvention with the resolved n_chi_eq.
  • geometries/schema.json: document the new field; clarify kappa_chi_eq's role.
  • geometries/{kappa4cv,kappa6c,kappa4ch}.yml: reword the kappa_chi_eq header comment to name only the kappa-arm tilt role.
  • geometries/__init__.py: distinguish the two axes in the package docstring.
  • kappa.py: KappaPseudoAngleConvention docstring — the rotation-matrix identity is solvable for any n_chi_eq perpendicular to n_komega, not only the in-plane choice.
  • forward.py: # pragma: no cover the unreachable degenerate branch of _solve_bisecting_analytic (no shipped geometry / mode / reflection lands there post-fix).

Test changes

  • New tests/test_regression_issue_284.py (67 cases):
    • Exact issue reproducer (sapphire reflections solve).
    • Cross-equivalence kappa4cv ↔ fourcv, kappa6c ↔ psic, kappa4ch ↔ fourch.
    • Q-equivalence sweep — restores the central invariant from the pre-BUG: geometry axis drawings for kappa and revision of underlying code #252 test_regression_issue_241.py file.
    • YAML auto-derivation, explicit override, numeric-vector form, invalid-form rejection, fallback when +longitudinal is parallel to n_komega, and GeometrySchemaError when no basis direction is perpendicular.
  • test_geometry_loader.py::test_kappa_chi_eq_numeric_vector_form — rewritten to assert the field's actual current role (kappa stage axis construction) plus the auto-derived n_chi_eq.
  • test_regression_issue_267.py — hand-built _reference_kappa4cv / _reference_kappa6c updated to n_chi_eq = +LONGITUDINAL (kappa4ch unchanged).
  • test_regression_issue_252.py::test_eulerian_kappa_round_trip — asserts the rotation-matrix identity rather than the angle identity (both branches are valid decompositions; geometry-dependent which branch the +1 parameter lands on).
  • test_factories.py::test_make_geometry_kappa_alpha_forwarded — docstring rewritten to distinguish the two fields.

Other

Local QC (all green)

  • pytest: 2571 passed, 2 skipped, 3 deselected; 100% line + branch coverage.
  • pytest -m slow_benchmark: 3/3 passed.
  • pre-commit run --all-files: clean.
  • US-English grep over every changed file: clean.
  • make -C docs clean html: succeeded, zero warnings, zero RST role leaks.

Notes on related issues

Contributed by: OpenCode (argo/claudeopus47)

… axis

Issue #284 reported that kappa4cv bisecting and kappa6c
bisecting_vertical lost solutions for sapphire asymmetric reflections
(0,1,2), (0,0,6), (1,1,3) in v0.11.0.

Root cause: issue #252 conflated two distinct concepts in the single
YAML field 'kappa_chi_eq':

1. The kappa-arm tilt direction used by Walko's formula
   'n_kappa = cos(α)*n_komega_unsigned + sin(α)*tilt_direction'.  For
   kappa4cv this is +vertical (kappa arm lies in the transverse-
   vertical plane).
2. The equivalent-Eulerian chi pseudo-angle axis the kappa-to-Eulerian
   decomposition rotates about for the virtual chi angle.  For kappa4cv
   this must be +longitudinal to match fourcv's chi axis (the
   non-kappa Eulerian preset kappa4cv is mechanically equivalent to).

Setting both to the same value (+vertical) broke the second role: the
kappa equivalent-Eulerian decomposition's reachable Bragg locus shrank
away from the locus reachable by fourcv bisecting, making asymmetric
reflections decline.

Implementation (Option C from the investigation):

- src/ad_hoc_diffractometer/geometry_loader.py:
  * Add an optional top-level YAML field 'kappa_eulerian_chi' that
    sets the convention's n_chi_eq verbatim when present.
  * When absent, derive n_chi_eq automatically as the first basis
    direction perpendicular to n_komega in the conventional order
    (+longitudinal, +vertical, +transverse) -- yields +longitudinal
    for every shipped kappa preset.
  * Helper _derive_kappa_eulerian_chi() implements the derivation.
- src/ad_hoc_diffractometer/geometries/schema.json: document the new
  field and clarify kappa_chi_eq's role.
- src/ad_hoc_diffractometer/geometries/{kappa4cv,kappa6c,kappa4ch}.yml:
  reword the kappa_chi_eq header comment to name only the kappa-arm
  tilt role; cite the new field for the eulerian-equivalent role.
- src/ad_hoc_diffractometer/geometries/__init__.py: rewrite the
  'Kappa angle (alpha) convention' section to distinguish the two
  axes; add a 'Equivalent-Eulerian chi axis vs kappa-arm tilt
  direction (issue #284)' subsection.
- src/ad_hoc_diffractometer/kappa.py: update KappaPseudoAngleConvention
  docstring -- the rotation-matrix identity is solvable for any
  n_chi_eq perpendicular to n_komega, not only the in-plane choice.
- src/ad_hoc_diffractometer/forward.py: pragma the unreachable
  degenerate branch of _solve_bisecting_analytic
  (w_perp_norm < 1e-12 or q_perp_norm < 1e-12) -- no shipped geometry,
  mode, or reflection lands there after the n_chi_eq correction.

Tests (per-test audit; no algorithmic test edits):

- tests/test_regression_issue_284.py (new): exact issue reproducer,
  cross-equivalence kappa<->fourcv/fourch/psic, kappa-arm-axis
  invariance, Q-equivalence sweep restored from the pre-#252
  test_regression_issue_241.py file, YAML auto-derivation /
  explicit-override / numeric-vector / invalid-form / fallback /
  no-perpendicular-basis cases.
- tests/test_geometry_loader.py::test_kappa_chi_eq_numeric_vector_form:
  rewritten to assert the kappa stage axis (the field's actual role)
  and the auto-derived n_chi_eq.
- tests/test_regression_issue_267.py: hand-built kappa4cv and kappa6c
  reference geometries updated to use n_chi_eq = +LONGITUDINAL
  (matching the new auto-derivation; kappa4ch already had the right
  value because tilt direction and eulerian chi coincide there).
- tests/test_regression_issue_252.py::test_eulerian_kappa_round_trip:
  asserts the rotation-matrix identity rather than the angle identity
  (each branch is a valid decomposition; +1 branch can land on the
  chi-mirrored angles for some kappa presets).
- tests/test_factories.py::test_make_geometry_kappa_alpha_forwarded:
  docstring rewritten to distinguish the two fields.

Other:

- .gitignore: broaden '.coverage' to '.coverage*' (covers context-
  enabled coverage runs and per-process suffixes).
- CHANGES.md: replace the bullet-style 'Unreleased' header with an
  HTML comment block (matches the user's CHANGES.md style preference).

Verification (local):
- 2571 passed, 2 skipped, 3 deselected; 100% line + branch coverage.
- slow_benchmark 3/3 passed.
- pre-commit clean.
- US-English grep clean over every changed file.
- Sphinx 'make clean html' build succeeded; zero warnings; zero RST
  role leaks (both grep patterns from AGENTS.md).

Contributed by: OpenCode (argo/claudeopus47)
@prjemian prjemian merged commit 476a8fe into main May 20, 2026
7 checks passed
@prjemian prjemian deleted the 284-kappa-eulerian-chi-axis branch May 20, 2026 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Regression: kappa4cv/kappa6c bisecting modes lose solutions for sapphire reflections in v0.11.0

1 participant