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

Calculation of SCF expansion based on an N-body representation of the density #444

Merged
merged 15 commits into from Feb 23, 2021

Conversation

morganb-phys
Copy link
Contributor

Introduce SCF expansion for n-body simulations. Added two new functions in SCFPotential.py for calculating SCF coefficients from an n-body simulation:
-scf_compute_coeffs_spherical_nbody
-scf_compute_coeffs_nbody

Also include a test for the spherical case. Non-spherical test to be added soon.

@jobovy jobovy changed the title Scfnbody Calculation of SCF expansion based on an N-body representation of the density Jan 12, 2021
@jobovy jobovy self-assigned this Jan 12, 2021
@jobovy
Copy link
Owner

jobovy commented Jan 12, 2021

Can you put the tests just in test_scf? I think it fails because HernquistPotential isn't imported.

@morganb-phys
Copy link
Contributor Author

I've deleted test_SCFPotential.py and added the test to test_scf.py

@codecov
Copy link

codecov bot commented Jan 13, 2021

Codecov Report

Merging #444 (b69966b) into master (9ac4b58) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #444   +/-   ##
=======================================
  Coverage   99.78%   99.78%           
=======================================
  Files         183      183           
  Lines       25635    25711   +76     
=======================================
+ Hits        25580    25656   +76     
  Misses         55       55           
Impacted Files Coverage Δ
galpy/potential/SCFPotential.py 99.75% <100.00%> (+0.05%) ⬆️
galpy/potential/__init__.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9ac4b58...b69966b. Read the comment docs.


r = numpy.sqrt(pos[0]**2+pos[1]**2+pos[2]**2)
phi = numpy.arctan2(pos[1],pos[0])
costheta = pos[2]/r
Copy link
Contributor

Choose a reason for hiding this comment

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

Astropy does not (yet) support the ufunc lpmv, so dimensionless quantities need to be recast.
The following works on both normal and astropy Quantity arrays.

costheta = u.Quantity(pos[2]/r, copy=False).to_value(u.one)

Copy link
Owner

Choose a reason for hiding this comment

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

To deal with positions given with units, we'll just have to apply the relevant decorator. I think probably the one used for the 2D potential functions should work?

Cn= numpy.zeros((1,len(r),N,L,1))
for i in range(N):
for j in range(L):
Cn[0,:,i,j,0]= C[i][j]((r/a-1)/(r/a+1))
Copy link
Contributor

@nstarman nstarman Feb 11, 2021

Choose a reason for hiding this comment

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

Likewise, eval_ gegenbauer is not an astropy-supported ufunc.

Ylm= numpy.nan_to_num(Ylm)

C= gegenbauer(nn,2.*ll+1.5)
Cn= C((r/a-1)/(r/a+1))
Copy link
Contributor

@nstarman nstarman Feb 11, 2021

Choose a reason for hiding this comment

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

Likewise, eval_ gegenbauer is not an astropy-supported ufunc.

for j,ll in enumerate(l):
for k,mm in enumerate(m[:j+1]):

Plm= lpmv(mm,ll,costheta)
Copy link
Contributor

@nstarman nstarman Feb 11, 2021

Choose a reason for hiding this comment

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

Astropy does not (yet) support the ufunc lpmv, so dimensionless quantities need to be recast.

@nstarman nstarman mentioned this pull request Feb 11, 2021
8 tasks
@jobovy jobovy changed the base branch from master to improvednfwdf February 18, 2021 17:32
@jobovy jobovy changed the base branch from improvednfwdf to master February 18, 2021 17:32
@jobovy jobovy merged commit dd84204 into jobovy:master Feb 23, 2021
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

3 participants