Skip to content

✨ feat(transforms): compose time-dependent rotations with @#585

Open
nstarman wants to merge 2 commits into
GalacticDynamics:mainfrom
nstarman:claude/v024-rotate-matmul-callable
Open

✨ feat(transforms): compose time-dependent rotations with @#585
nstarman wants to merge 2 commits into
GalacticDynamics:mainfrom
nstarman:claude/v024-rotate-matmul-callable

Conversation

@nstarman

Copy link
Copy Markdown
Contributor

Summary

Rotate.__matmul__ raised NotImplementedError whenever either operand had a callable (time-dependent) rotation matrix:

spin = cxfm.Rotate.from_(R_of_t)
spin @ spin   # NotImplementedError: @ is not yet implemented for Rotate with callable R.

so rotating frames could only be combined via |/Composed, not @. Surfaced by the v0.24 pre-release audit.

Fix

Compose pointwise in tau: op1 @ op2 yields a Rotate whose matrix is op2.R(τ) @ op1.R(τ), matching the existing constant-matrix convention (op1 @ op2).R == op2.R @ op1.R. A small ComposedR wrapper (alongside the existing Neg) evaluates the two operands — each a constant matrix or a callable of τ — and returns their product, so the result materializes/JITs correctly and is reported as time-dependent. Mixed callable/constant operands work too.

Verification

  • Composition matches the pointwise product op2.R(τ) @ op1.R(τ).
  • Acting with op1 @ op2 equals applying op1 then op2.
  • is_time_dependent(op1 @ op2) is True; constant @ unchanged.
  • New unit tests (test_simplify.py) + a doctest; tests/unit/transforms passes (264).

Built on current main (post-#571, unxt v2.0).

🤖 Generated with Claude Code

`Rotate.__matmul__` raised `NotImplementedError` whenever either operand had a
callable (time-dependent) rotation matrix, so rotating frames could not be
combined with `@` (only via `|`/`Composed`).

Compose pointwise in tau: `(op1 @ op2)` yields a `Rotate` whose matrix is
`op2.R(tau) @ op1.R(tau)`, matching the constant-matrix convention
`(op1 @ op2).R == op2.R @ op1.R`. A small `ComposedR` wrapper (alongside `Neg`)
evaluates the two operands — each a constant matrix or a callable — and returns
their product, so the result is materialize/jit-friendly and correctly reported
as time-dependent. Mixed callable/constant operands work too.

Verified: composition matches the pointwise product; acting with `op1 @ op2`
equals applying op1 then op2; `is_time_dependent` is True. New unit tests +
doctest.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 25, 2026 02:27
@github-actions github-actions Bot added ✅ Add / update / pass tests Add, update, or pass tests. ✨ Introduce new features Introduce new features. labels Jul 25, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Enables Rotate.__matmul__ (@) for time-dependent (callable) rotation matrices by composing the two rotations pointwise in tau, so rotating frames can be combined with @ (not only via |/Composed) while preserving the existing constant-matrix convention.

Changes:

  • Add ComposedR wrapper to represent the pointwise product of two rotation parameters (constant or callable).
  • Update Rotate.__matmul__ to return a time-dependent Rotate when either operand’s R is callable.
  • Add unit tests covering callable/callable and callable/constant @ composition semantics.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/coordinax/transforms/_src/actions/utils.py Adds ComposedR helper used to compose constant/callable rotation parameters uniformly.
src/coordinax/transforms/_src/actions/rotate.py Implements callable-aware Rotate.__matmul__ using ComposedR and adds a doctest example.
tests/unit/transforms/test_simplify.py Adds tests validating pointwise composition and action equivalence for callable rotations.

Comment thread tests/unit/transforms/test_simplify.py
@codecov

codecov Bot commented Jul 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.69%. Comparing base (b6aa5dd) to head (73b298a).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #585      +/-   ##
==========================================
+ Coverage   94.66%   94.69%   +0.03%     
==========================================
  Files         247      247              
  Lines        7894     7904      +10     
==========================================
+ Hits         7473     7485      +12     
+ Misses        421      419       -2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@nstarman nstarman added this to the v0.24.0 milestone Jul 25, 2026
Add a `jax.jit` test that materializes a composed time-dependent
rotation, per PR review feedback that new callable-`@` behavior should
be validated under JAX transformations.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 25, 2026 14:52

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

✅ Add / update / pass tests Add, update, or pass tests. ✨ Introduce new features Introduce new features.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants