📝 docs(spec): specify the lorentzian verbs - #734
Merged
nstarman merged 3 commits intoAug 18, 2026
Conversation
github-actions
Bot
added
the
📝 Add / update documentation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the repository’s authoritative specification (docs/spec.md) to formally specify the coordinax.manifolds.lorentzian namespace and its four public verbs, including signatures, semantics, dispatch expectations, and executable examples.
Changes:
- Adds a new spec section for
coordinax.manifolds.lorentzian, documenting the Lorentzian-signature gate and listing the supported verbs. - Specifies behavior for
causal_character,proper_time,proper_distance, andrapidity_between, each with examples intended to run under the project’s doctest/Sybil harness.
Suppressed comments (1)
docs/spec.md:3178
proper_distancealso accepts anatolkeyword argument (same causal-class tolerance logic asproper_time), but the spec signatures omit it. The signatures should includeatolto match the implemented API.
```
cxm.lorentzian.proper_distance(chart, a, b, /, *, usys=None)
cxm.lorentzian.proper_distance(metric, chart, a, b, /, *, usys=None)
```
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #734 +/- ##
==========================================
+ Coverage 96.56% 96.59% +0.03%
==========================================
Files 266 266
Lines 8852 8934 +82
==========================================
+ Hits 8548 8630 +82
Misses 304 304 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
nstarman
force-pushed
the
claude/spec-lorentzian-verbs
branch
from
August 18, 2026 22:24
d220e5b to
7351a70
Compare
The `coordinax.manifolds.lorentzian` sub-namespace had no entry in the spec at all -- not the namespace, and none of its four verbs. `causal_character` and `rapidity_between` were not so much as mentioned; `proper_time` and `proper_distance` appeared once each, inside `geodesic_distance`'s "use these instead" line. Five sections: the namespace, then a verb each. The namespace section carries what they share, so the verb entries stay short -- the gate is `AbstractLorentzianMetricField`, meaning *exactly one* timelike direction, and the reason each refuses rather than returning something readable as a classification. Also why the namespace is named for the signature: `galileanct` is a 4-dimensional spacetime and is not Lorentzian, so a `spacetime` namespace would promise membership these verbs refuse, while a `minkowski` one would exclude Schwarzschild and FLRW, which carry the marker and acquire all four. Worth pinning, and now pinned: - `causal_character` returns a Python `str`, not an enum and not a number, so it cannot be accidentally arithmetic. It takes `atol`, because a null separation is measure-zero in floating point and needs a band rather than an equality test. - `proper_time` divides by `c` and `proper_distance` does not, so one is a duration and the other a length out of the same length-squared interval. - Each refuses the other's causal class with `ValueError` rather than `nan`: no clock travels between spacelike-separated events, so the question does not apply rather than being undefined. - `rapidity_between` is the odd one -- tangents rather than points, `at=` required because there is no first point to take the base from, and a bare dimensionless `Array` rather than an `Angle`, since rapidity is unbounded and `rad` would wrongly suggest it wraps. It exists because `angle_between` refuses timelike pairs correctly: both `g(u,u)` and `g(v,v)` are negative, so `arccos` would clip and report two observers in relative motion as parallel. All 34 examples were executed under the harness that will run them, not a bare interpreter. That distinction is load-bearing here: `JAX_ENABLE_X64 = "1"` is set in `[tool.pytest.ini_options]`, so an ad-hoc `python` session yields `float32` and every literal would have been wrong -- `Q(1.6341238e-08, 's')` against the `Q(1.63412366e-08, 's')` the suite actually produces. Explanatory prose sits outside the `pycon` fences rather than inside them, per the review on GalacticDynamics#733: everything within a fence renders as code, taking its inline markup with it. The 17 pre-existing failures elsewhere in the file are GalacticDynamics#733's subject. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three inaccuracies from review, each checked against the implementation:
`causal_character` was described as symmetric in `a` and `b`. It is the sign of
`interval`, which evaluates the metric at the *first* point, so the guarantee
only holds where the metric is constant along the path. That covers everything
shipped -- `MinkowskiMetric` is the sole Lorentzian metric and it is flat -- but
not the curved case the namespace explicitly invites, where the module docstring
promises Schwarzschild and FLRW inherit these verbs. Reworded to say where the
symmetry holds and to warn against relying on it elsewhere.
`proper_time` and `proper_distance` both accept `atol` and neither signature
listed it. It is the same band `causal_character` uses, forwarded through, so it
is what decides how near null a pair may be before being refused -- worth
stating, since it is the knob that moves the refusal boundary.
`rapidity_between` was documented as raising `ValueError` on a non-timelike
operand. True eagerly; under `jax.jit` the condition is traced, so the same test
becomes a mask and the result is `nan`:
eager -> ValueError: ... only between two timelike tangent vectors
jitted -> Array(nan, dtype=float64)
Both behaviours are now stated, with the instruction to check the output rather
than expect the exception inside a traced function.
That last one generalises, so the namespace section gains a **Tracing** note:
the three causal verbs return or branch on a Python value -- a `str`, or an
exception chosen by the class -- and so are eager-only, raising
`ConcretizationTypeError` under `jit`. `rapidity_between` is the exception, and
is the only one of the four that traces.
334 passed; `prek run --all-files` clean; docs build succeeds.
nstarman
force-pushed
the
claude/spec-lorentzian-verbs
branch
from
August 18, 2026 22:49
7351a70 to
c40add8
Compare
nstarman
added a commit
to nstarman/coordinax
that referenced
this pull request
Aug 18, 2026
Audit prompted by the review on GalacticDynamics#734, whose middle finding was a signature that omitted a keyword the implementation accepts. Checking these four the same way turned up the same class of error and one wrong rule. `pt_project` has seven overloads and the spec listed five. The two missing are the explicit-embedding-map form and the `(from_chart, EmbeddedChart)` form, and the `Point` overload takes `usys` like every other -- it was written as `pt_project(p, M, /)`, promising a narrower API than exists. The diagonal rule was stated as a property of orthogonality and is not. It is declared per (manifold, chart) pair, and the counter-example is on the sphere already: S2 / sph2 DiagonalMetric S2 / lonlat_sph2 DiagonalMetric S2 / loncoslat_sph2 DenseMetric Reworded to say the chart decides, with `loncoslat_sph2` shown as a worked example rather than left as a trap for whoever assumed the manifold decides. Also pinned what the `(AbstractManifold, AbstractChart)` fallback actually returns -- `DenseMetric`, verified -- and why that is the right default: dense is only slower, diagonal would be wrong. 331 passed; `prek run --all-files` clean; docs build succeeds.
Self-review pass for redundancy. 290 lines to 276, with no contract dropped. `A non-Lorentzian metric raises NotImplementedError` appeared as a bullet under all four verbs, and the namespace section above already states it for all four. Said once there, in bold, and left out of the verb entries; the refusal is still pinned by the executable example under `causal_character`. The paragraph re-deriving the gate restated the two sentences directly above it. The `rapidity_between` preamble spent ten lines arguing why `angle_between` cannot serve, which needs four. `proper_time`'s return bullet re-derived `tau = sqrt(-ds^2) / c` from the formula three lines above. `causal_character` described `metric` once under Arguments and again at length under Dispatch. Also folded away a `pycon` fence that held three imports and nothing else, immediately followed by another fence -- an artifact of moving prose out of the blocks, not a deliberate split. Left alone: every example. They are the executable checks, and the three refusal cases are exactly what would catch this contract drifting now that the file gates CI. Also left the per-section import preambles, which duplicate each other but match how the rest of the file is written. 334 passed; `prek run --all-files` clean; docs build succeeds.
nstarman
added a commit
to nstarman/coordinax
that referenced
this pull request
Aug 18, 2026
Audit prompted by the review on GalacticDynamics#734, whose middle finding was a signature that omitted a keyword the implementation accepts. Checking these four the same way turned up the same class of error and one wrong rule. `pt_project` has seven overloads and the spec listed five. The two missing are the explicit-embedding-map form and the `(from_chart, EmbeddedChart)` form, and the `Point` overload takes `usys` like every other -- it was written as `pt_project(p, M, /)`, promising a narrower API than exists. The diagonal rule was stated as a property of orthogonality and is not. It is declared per (manifold, chart) pair, and the counter-example is on the sphere already: S2 / sph2 DiagonalMetric S2 / lonlat_sph2 DiagonalMetric S2 / loncoslat_sph2 DenseMetric Reworded to say the chart decides, with `loncoslat_sph2` shown as a worked example rather than left as a trap for whoever assumed the manifold decides. Also pinned what the `(AbstractManifold, AbstractChart)` fallback actually returns -- `DenseMetric`, verified -- and why that is the right default: dense is only slower, diagonal would be wrong. 331 passed; `prek run --all-files` clean; docs build succeeds.
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.
coordinax.manifolds.lorentzianhad no entry in the spec at all — not the namespace, and none of its four verbs.causal_characterandrapidity_betweenwere not so much as mentioned;proper_timeandproper_distanceappeared once each, insidegeodesic_distance's "use these instead" line.Five sections: the namespace, then a verb each. The namespace section carries what they share, so the verb entries stay short.
causal_characterintervalproper_timeproper_distancerapidity_betweenWhat's worth pinning
causal_characterreturns a Pythonstr— not an enum, not a number, so it cannot be accidentally arithmetic. It takesatol, because a null separation is measure-zero in floating point and needs a band rather than an equality test.proper_timedivides byproper_distancedoes not, so one is a duration and the other a length out of the same length-squared interval.ValueError, notnan. No clock travels between spacelike-separated events, so the question does not apply rather than being undefined.rapidity_betweenis the odd one out and gets the most prose: tangents rather than points,at=required because there is no first point to take the base from, and a bare dimensionlessArrayrather than anAngle— rapidity is unbounded, andradwould wrongly suggest it wraps.That last one also gets the why it exists:$g(u,u)$ and $g(v,v)$ are both negative, so $0$ or $\pi$ and reports two observers in relative motion as parallel. The invariant that separates them is hyperbolic, so it gets its own name rather than a branch inside a circular-angle function.
angle_betweenrefuses timelike pairs, and correctly —arccosof their ratio clips toThe namespace section also records why it is named for the signature:
charts.galileanctis a 4-dimensional spacetime and is not Lorentzian, so aspacetimenamespace would promise membership these verbs refuse;minkowskiwould wrongly exclude Schwarzschild and FLRW, which carry the marker and acquire all four.Verification — and one trap worth naming
All 34 examples were executed under the harness that will run them, not a bare interpreter.
That distinction is load-bearing.
JAX_ENABLE_X64 = "1"is set in[tool.pytest.ini_options], so an ad-hocpythonsession givesfloat32and every literal would have been wrong:I gathered the first set before noticing, and every one of them would have failed CI. Verified with
pytest docs/spec.md: 34 collected, 34 passed, 0 failed in the added range.prek run --all-filesclean (blacken-docsreformatted; re-verified after, still 0 failing).uv run --frozen nox -s docssucceeds with warnings as errors.The 17 pre-existing failures elsewhere in
docs/spec.mdare untouched here — they are #733's subject.Stacking
Touches
docs/spec.md, as do #732 and #733, and all three insert near the end of the verb cluster. They are independent; whichever lands last needs a trivial rebase. Once #733 is in, these examples are CI-gated automatically.🤖 Generated with Claude Code