✨ feat(transforms): compose time-dependent rotations with @#585
Open
nstarman wants to merge 2 commits into
Open
✨ feat(transforms): compose time-dependent rotations with @#585nstarman wants to merge 2 commits into
nstarman wants to merge 2 commits into
Conversation
`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>
Contributor
There was a problem hiding this comment.
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
ComposedRwrapper to represent the pointwise product of two rotation parameters (constant or callable). - Update
Rotate.__matmul__to return a time-dependentRotatewhen either operand’sRis 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. |
Codecov Report✅ All modified and coverable lines are covered by tests. 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. 🚀 New features to boost your workflow:
|
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Rotate.__matmul__raisedNotImplementedErrorwhenever either operand had a callable (time-dependent) rotation matrix:so rotating frames could only be combined via
|/Composed, not@. Surfaced by the v0.24 pre-release audit.Fix
Compose pointwise in
tau:op1 @ op2yields aRotatewhose matrix isop2.R(τ) @ op1.R(τ), matching the existing constant-matrix convention(op1 @ op2).R == op2.R @ op1.R. A smallComposedRwrapper (alongside the existingNeg) 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
op2.R(τ) @ op1.R(τ).op1 @ op2equals applyingop1thenop2.is_time_dependent(op1 @ op2)isTrue; constant@unchanged.test_simplify.py) + a doctest;tests/unit/transformspasses (264).Built on current
main(post-#571, unxt v2.0).🤖 Generated with Claude Code