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

Analytic Einasto and Hernquist normalization #956

Merged
merged 8 commits into from
Jul 21, 2022
Merged

Conversation

hsinfan1996
Copy link
Contributor

@hsinfan1996 hsinfan1996 commented Jul 20, 2022

Some functions in src/ccl_haloprofile.c and pyccl/ccl_haloprofile.i are not needed anymore, but I did not touch them.

@hsinfan1996 hsinfan1996 marked this pull request as draft July 20, 2022 14:27
@coveralls
Copy link

coveralls commented Jul 20, 2022

Pull Request Test Coverage Report for Build 2709235802

  • 7 of 7 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.002%) to 97.1%

Totals Coverage Status
Change from base Build 2669526928: -0.002%
Covered Lines: 4553
Relevant Lines: 4689

💛 - Coveralls

@damonge
Copy link
Collaborator

damonge commented Jul 20, 2022

Thanks a lot @hsinfan1996 . Can you check how much faster (or slower) it is to call the gamma functions than computing the normalisation numerically?

@hsinfan1996
Copy link
Contributor Author

Thanks a lot @hsinfan1996 . Can you check how much faster (or slower) it is to call the gamma functions than computing the normalisation numerically?

  1. If M, R_s, R_M and alpha are floats, ~9 times faster.
  2. If M, R_s, R_M and alpha are arrays with each having only one element, ~1.2 times faster.
  3. If M, R_s, R_M and alpha are arrays with each having 100 elements, ~7 times faster.

@hsinfan1996 hsinfan1996 changed the title Analytic Einasto normalization Analytic Einasto and Hernquist normalization Jul 20, 2022
@damonge
Copy link
Collaborator

damonge commented Jul 20, 2022

OK, awesome, in that case feel free to remove all traces of the old functions in src/ccl_haloprofile.c and pyccl/ccl_haloprofile.i

@hsinfan1996 hsinfan1996 marked this pull request as ready for review July 20, 2022 17:41
@hsinfan1996
Copy link
Contributor Author

hsinfan1996 commented Jul 20, 2022

Coveralls is down. Could not submit results when the check for macos finished.

@hsinfan1996 hsinfan1996 requested a review from damonge July 20, 2022 18:03
@damonge
Copy link
Collaborator

damonge commented Jul 20, 2022

Ignore coveralls. Coverage went down because you reduced the number of lines in the code, I bet.

Copy link
Collaborator

@damonge damonge left a comment

Choose a reason for hiding this comment

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

Thanks a lot! It feels great to get rid of unnecessary C code!
Just one comment.

include/ccl.h Show resolved Hide resolved
@damonge
Copy link
Collaborator

damonge commented Jul 20, 2022

It seems coveralls is undergoing some maintenance. I'm sure it's irrelevant, but let's wait for tomorrow before merging (pressing the red merge button makes my brain scream a bit)

@hsinfan1996
Copy link
Contributor Author

It seems coveralls is undergoing some maintenance. I'm sure it's irrelevant, but let's wait for tomorrow before merging (pressing the red merge button makes my brain scream a bit)

Sure. I want it to pass all the checks too. I will re-run the jobs when Coveralls is stable.

Copy link
Collaborator

@damonge damonge left a comment

Choose a reason for hiding this comment

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

LGTM!

@damonge damonge merged commit 77624d2 into master Jul 21, 2022
@damonge damonge deleted the analytic_einasto_norm branch July 21, 2022 08:28
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.

Analytic expression for the normalization of Einasto and Hernquist profiles
3 participants