Skip to content

MultipoleProfilePotential: replace the ad-hoc symmetry string with the planned symmetry flag (#189) #854

Description

@nstarman

Follow-up to #852, and a concrete first consumer for #189.

The problem

MultipoleProfilePotential (#852) is the first potential in galax that
carries an explicit symmetry, and it does so with an ad-hoc stringly-typed
field:

symmetry: str | None = field(static=True, default=None)

with the accepted values None, "spherical", "axisymmetric",
"triaxial" interpreted by a single function, lm_keys(l_max, symmetry),
which returns the (l, m) modes that can be non-zero. Anything else raises
ValueError("Unknown symmetry ...").

That works for the one thing it was built for — pruning modes that vanish —
but it is the wrong long-term shape:

  • Stringly typed. No IDE completion, no static checking, typos caught
    only at runtime and only inside one function.
  • It encodes exactly one consequence of symmetry. Mode pruning. It says
    nothing about input types, evaluation shortcuts, or composition.
  • It is a second, parallel notion of symmetry to whatever Support symmetries in potentials #189 lands, in
    a package that will then have two.
  • The taxonomy is already awkward. "triaxial" here means "octant
    symmetry" — even l, even m >= 0 — i.e. reflection symmetry in all
    three planes. That is a stronger statement than "triaxial" usually means,
    and it is not the same axis as "axisymmetric", which after ✨ feat(potential): add MultipoleProfilePotential #852 correctly
    keeps all l at m = 0 (axisymmetry constrains m, not l; assuming
    it also kills odd l was a real bug, fixed in that PR). The two options
    are not points on one scale, and a flag makes that explicit where a string
    hides it.

What #189 would give it

#189 proposes that potentials know their symmetry so that, for example, a
spherical potential can be called with a 1-D radius and the machinery
promotes it to 3-D Cartesian internally. Composed as a flag rather than an
enum of named cases, the same declaration can drive several things at once:

  • which (l, m) modes can be non-zero (what lm_keys does today);
  • which input types are safe — r for spherical, (R, z) for axisymmetric,
    xyz otherwise — and how to promote them;
  • evaluation shortcuts (a spherical expansion needs no harmonic table at
    all);
  • how symmetry composes under the xfm wrappers, which currently can silently
    destroy it: TriaxialInThePotential applied to a spherical base is no
    longer spherical, and nothing today notices.

Reflection symmetry about each plane, azimuthal symmetry and full spherical
symmetry are independent bits, which is what makes a flag the right
representation and what the current four-string enumeration cannot express.

The migration for this PR's code

Once #189 exists:

  1. Replace symmetry: str | None with the flag type.
  2. Reimplement lm_keys(l_max, symmetry) against it. The current mapping is
    the specification to preserve:
    • no symmetry -> all (l, m), -l <= m <= l
    • spherical -> (0, 0) only
    • axisymmetric -> m = 0, all l
    • octant/triaxial -> even l, even m >= 0
  3. Have from_density / from_potential take the flag, and — where the
    source is itself a galax potential — default to its declared symmetry
    rather than None. Today the caller must supply it by hand and gets a
    full expansion if they forget, which is correct but wasteful.
  4. Keep __check_init__'s consistency check (lm_keys == lm_keys(l_max, symmetry)), retargeted at the flag.

Notes

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions