potential/backend: anchor scalar coordinates in migrated analytic/disk compute methods#992
Open
jobovy wants to merge 1 commit into
Open
potential/backend: anchor scalar coordinates in migrated analytic/disk compute methods#992jobovy wants to merge 1 commit into
jobovy wants to merge 1 commit into
Conversation
…k compute methods The migrated compute methods of these potentials feed coordinate operands straight to xp.<fn> (xp.cos/sin/exp/sqrt/log/abs/sign), which torch rejects on a python-float / numpy.float64 / numpy.ndarray. #990's potential_physical_input boundary coercion covers the public array-evaluation path for backend-compatible potentials, but it is BYPASSED by (a) the numerical-derivative mixin and other DIRECT private-method calls (test_2ndDeriv / test_forceAsDeriv), (b) the internal axisymmetric phi=0 reset (mockFlatEllipticalDisk), and (c) parameter/t forwarding (KuijkenDubinski). Coerce the operands at the start of each compute method via coerce_coords -- the established per-method convention the already-migrated potentials use (e.g. EllipsoidalPotential._anchor_phi). Flips 6 ledgered torch entries that bypass the #990 boundary: test_2ndDeriv_potential[CorotatingRotation3DSpiralArmsPotential] test_forceAsDeriv_potential[CorotatingRotation3DSpiralArmsPotential] test_2ndDeriv_potential[mockFlatEllipticalDiskPotential] test_amp_mult_divide[mockFlatEllipticalDiskPotential] test_forceAsDeriv_potential[mockFlatEllipticalDiskPotential] test_kuijkendubinski_t_forwarding Also brings PowerSpherical / PowerSphericalwCutoff to the same per-method coerce_coords parity (migrated before the convention existed). These flip no currently-red entry -- #990's boundary masks their direct-compute-method gap -- but the fix closes that gap and is byte-identical, so it is convention-parity hardening rather than a red-flip. numpy path is byte-identical (coerce_coords is an object-identical pass-through when xp is numpy; verified identical output hashes across the 5 files). jax value-identical (25 entries pass under jax). The new coerce_coords lines execute on the numpy test_potential path (coverage-uploaded). No ledger change (the 6 become XPASS, green via strict=False; pruned in the separate ledger-regen PR). Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## feat/backends #992 +/- ##
==============================================
Coverage 99.93% 99.93%
==============================================
Files 254 254
Lines 39839 39897 +58
Branches 839 839
==============================================
+ Hits 39813 39871 +58
Misses 26 26 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
What
Anchor scalar/numpy coordinate operands onto the active backend at the start of the migrated compute methods of 5 potentials (
SpiralArmsPotential,EllipticalDiskPotential,KuijkenDubinskiDiskExpansionPotential,PowerSphericalPotential,PowerSphericalPotentialwCutoff), via the established per-methodcoerce_coordsconvention (the same oneEllipsoidalPotential._anchor_phiuses).These methods feed coordinates straight to
xp.<fn>(cos/sin/exp/sqrt/log/abs/sign), which torch rejects on apython-float/numpy.float64/numpy.ndarray. #990'spotential_physical_inputboundary coercion covers the public array-evaluation path for backend-compatible potentials, but it is bypassed by:test_2ndDeriv/test_forceAsDerivnumerical-derivative path through undecorated mock wrappers),phi = 0reset (mockFlatEllipticalDisk),tforwarding (kuijkendubinski_t_forwarding).Result — 6 genuine torch reds flipped
(verified fail-without / pass-with under forced torch, regen mode.)
PowerSpherical/PowerSphericalwCutoffflip no currently-red entry — #990's boundary masks their direct-compute-method gap — but they were migrated before this convention existed, so they're brought to per-methodcoerce_coordsparity (byte-identical; closes the latent direct-call torch gap). Honest defense-in-depth, not a red-flip.Safety (adversarially reviewed — two independent passes)
coerce_coordsis an object-identical pass-through whenxp is numpy; verified identical SHA-256 output hashes across all 5 families (incl. the KuijkenDubinski lambda→def closure conversion, which is behaviorally identical).3.95e-14across the 5 families.coerce_coordslines execute on the numpytest_potentialpath (build.yml numpy jobs, coverage-uploaded); no backend-only line.strict=False; pruned in the separate ledger-regen PR).File-disjoint from #991 (
_coerce.py/_namespaces.py). Base is current feat/backends (#990).Out of scope (separate follow-ups the review surfaced)
expwholeDiskMultipoleExpansionPotential2ndDeriv — a construction-timenumpy.ndarray * TensorinMultipoleExpansionPotential.from_density(Finalize pvz etc. functions #75-adjacent).specialSpiralArmsPotential2ndDeriv — a torch finite-difference precision mismatch, not a scalar crash._backend_compatiblemock-bypass cluster (DehnenBar / Ferrers / SoftenedNeedleBar / …) — same vector, each needs its own per-method audit.🤖 Generated with Claude Code