Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: multipole potentials #357

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

nstarman
Copy link
Contributor

@nstarman nstarman commented Jun 19, 2024

Multipole potentials. Inner, Outer, & Combined.

This currently does not support time dependence on S/Tlm, but that's entirely because of the __check_init__.

@nstarman nstarman added this to the v0.1 milestone Jun 19, 2024
@nstarman nstarman force-pushed the pot-multipole branch 2 times, most recently from 32646b6 to c11d4e6 Compare June 25, 2024 02:50
@nstarman nstarman force-pushed the pot-multipole branch 2 times, most recently from d6324ca to 01099dd Compare June 30, 2024 17:13
Copy link

codecov bot commented Jun 30, 2024

Codecov Report

Attention: Patch coverage is 88.27586% with 17 lines in your changes missing coverage. Please review.

Project coverage is 89.96%. Comparing base (5a01695) to head (54d9d60).

Files Patch % Lines
src/galax/_galax_interop_gala/potential.py 0.00% 17 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #357      +/-   ##
==========================================
- Coverage   94.68%   89.96%   -4.72%     
==========================================
  Files          70       71       +1     
  Lines        2500     2642     +142     
==========================================
+ Hits         2367     2377      +10     
- Misses        133      265     +132     

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

@nstarman nstarman requested a review from adrn June 30, 2024 17:22
@nstarman nstarman marked this pull request as ready for review June 30, 2024 17:23
@nstarman nstarman mentioned this pull request Jun 30, 2024
83 tasks
@nstarman nstarman force-pushed the pot-multipole branch 3 times, most recently from a9099a3 to d12d2f1 Compare July 1, 2024 01:17
gala: gp.MultipolePotential, /
) -> gpx.MultipoleInnerPotential | gpx.MultipoleOuterPotential | gpx.PotentialFrame:
params = gala.parameters
cls = (
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Given that there's no combined inner/outer multipole expansion in Gala, this wasn't being tested by the automatic testing framework. I'll write a specific test.

@nstarman
Copy link
Contributor Author

nstarman commented Jul 1, 2024

@adrn, I think there are more efficient ways to compute this, but this implementation is already quite performant both during and post JIT. I think further efficiencies and code consolidation can happen in a followup PR.
Also, I'm very interested (in a followup PR) to implement sparsity support for S/T_lm!

Signed-off-by: nstarman <nstarman@users.noreply.github.com>
Signed-off-by: nstarman <nstarman@users.noreply.github.com>
Signed-off-by: nstarman <nstarman@users.noreply.github.com>
@nstarman
Copy link
Contributor Author

nstarman commented Jul 3, 2024

That's a strange one. It was passing. Shouldn't be failing now!
Also, it passes on my laptop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant