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

speed up chabrier1998 screening by adding `` entries #1396

Closed
zingale opened this issue Nov 26, 2023 · 3 comments
Closed

speed up chabrier1998 screening by adding `` entries #1396

zingale opened this issue Nov 26, 2023 · 3 comments

Comments

@zingale
Copy link
Member

zingale commented Nov 26, 2023

Currently, in chabrier1998, we do:

  Real Gamma1 = Gamma_e * std::pow(scn_fac.z1, 5.0_rt/3.0_rt);                                                                                          
  Real Gamma2 = Gamma_e * std::pow(scn_fac.z2, 5.0_rt/3.0_rt);                                                                                          
  Real Gamma12 = Gamma_e * std::pow(zcomp, 5.0_rt/3.0_rt);   

we should move these pow's into screen_factors_t, perhaps only enabled if we are using this screening routine. This will save on a lot of expensive pows

@yut23
Copy link
Collaborator

yut23 commented Nov 28, 2023

The quantum correction terms also use a lot of pows, which we should probably switch to nested evaluation like in screen5.

@zhichen3
Copy link
Collaborator

I'm actually debating on whether those terms are needed. These quantum correction terms were not in the original chabrier&potekhin paper. They were introduced in Calder's paper, so I added them. Removing them might give better convergence with NSE states too, although I never tested it.

zingale pushed a commit that referenced this issue Jan 1, 2024
added a runtime parameter to make quantum correction terms in chabrier1998 screening optional. This is because in NSE_solver quantum terms are not included.

Also fixed some inconsistent indentations

This should also speed up this screening a little bit (issue #1396), although we already switched integer pows to powi.
@zingale
Copy link
Member Author

zingale commented Jan 3, 2024

I think that this has been addressed now

@zingale zingale closed this as completed Jan 3, 2024
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

No branches or pull requests

3 participants