-
Notifications
You must be signed in to change notification settings - Fork 68
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
Allow Separable Growth Functions to compute Isotropized Power Spectra #1216
Conversation
Pull Request Test Coverage Report for Build 12161562031Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
pyccl/halos/pk_4pt.py
Outdated
@@ -669,6 +670,9 @@ def halomod_trispectrum_2h_22(cosmo, hmc, k, a, prof, *, prof2=None, | |||
p_of_k_a (:class:`~pyccl.pk2d.Pk2D`): a `Pk2D` object to | |||
be used as the linear matter power spectrum. If `None`, the power | |||
spectrum stored within `cosmo` will be used. | |||
separable_growth (Boolean): Indeicates whether a separable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: Indeicates -> Indicates and in all following occurrences.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and Boolean -> bool for consistency (and so the docs look nice)
pyccl/halos/pk_4pt.py
Outdated
@@ -976,6 +985,9 @@ def halomod_trispectrum_3h(cosmo, hmc, k, a, prof, *, prof2=None, | |||
p_of_k_a (:class:`~pyccl.pk2d.Pk2D`): a `Pk2D` object to | |||
be used as the linear matter power spectrum. If `None`, the power | |||
spectrum stored within `cosmo` will be used. | |||
separable_growth (Boolean): Indeicates whether a separable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: Indeicates -> Indicates
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Boolean -> bool
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great @paulrogozenski ! Just a few typos (thanks @rreischke )
@@ -1303,6 +1335,9 @@ def halomod_Tk3D_2h(cosmo, hmc, | |||
use_log (bool): if `True`, the trispectrum will be | |||
interpolated in log-space (unless negative or | |||
zero values are found). | |||
separable_growth (Boolean): Indeicates whether a separable | |||
growth function approximation can be used to calculate | |||
the isotropized power spectrum. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeicates -> Indicates
Boolean -> bool
@@ -1396,6 +1432,9 @@ def halomod_Tk3D_3h(cosmo, hmc, | |||
use_log (bool): if `True`, the trispectrum will be | |||
interpolated in log-space (unless negative or | |||
zero values are found). | |||
separable_growth (Boolean): Indeicates whether a separable | |||
growth function approximation can be used to calculate | |||
the isotropized power spectrum. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indicates, bool
@@ -1469,6 +1509,9 @@ def halomod_Tk3D_4h(cosmo, hmc, | |||
use_log (bool): if `True`, the trispectrum will be | |||
interpolated in log-space (unless negative or | |||
zero values are found). | |||
separable_growth (Boolean): Indeicates whether a separable | |||
growth function approximation can be used to calculate | |||
the isotropized power spectrum. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indicates, bool
pyccl/halos/pk_4pt.py
Outdated
@@ -669,6 +670,9 @@ def halomod_trispectrum_2h_22(cosmo, hmc, k, a, prof, *, prof2=None, | |||
p_of_k_a (:class:`~pyccl.pk2d.Pk2D`): a `Pk2D` object to | |||
be used as the linear matter power spectrum. If `None`, the power | |||
spectrum stored within `cosmo` will be used. | |||
separable_growth (Boolean): Indeicates whether a separable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and Boolean -> bool for consistency (and so the docs look nice)
pyccl/halos/pk_4pt.py
Outdated
@@ -976,6 +985,9 @@ def halomod_trispectrum_3h(cosmo, hmc, k, a, prof, *, prof2=None, | |||
p_of_k_a (:class:`~pyccl.pk2d.Pk2D`): a `Pk2D` object to | |||
be used as the linear matter power spectrum. If `None`, the power | |||
spectrum stored within `cosmo` will be used. | |||
separable_growth (Boolean): Indeicates whether a separable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Boolean -> bool
pyccl/halos/pk_4pt.py
Outdated
@@ -1140,6 +1159,9 @@ def halomod_trispectrum_4h(cosmo, hmc, k, a, prof, prof2=None, prof3=None, | |||
p_of_k_a (:class:`~pyccl.pk2d.Pk2D`): a `Pk2D` object to | |||
be used as the linear matter power spectrum. If `None`, the power | |||
spectrum stored within `cosmo` will be used. | |||
separable_growth (Boolean): Indeicates whether a separable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Boolean -> bool
@@ -1553,6 +1598,9 @@ def halomod_Tk3D_cNG(cosmo, hmc, prof, prof2=None, prof3=None, prof4=None, | |||
use_log (bool): if `True`, the trispectrum will be | |||
interpolated in log-space (unless negative or | |||
zero values are found). | |||
separable_growth (Boolean): Indeicates whether a separable | |||
growth function approximation can be used to calculate | |||
the isotropized power spectrum. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indicates, bool
Also, we need to add a unit test for this. Checking that you get the same result for some of this functions with or without scaling would be enough. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, apart from the typo (as noted by @damonge ).
If the results agree, we can merge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks Paul!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
A large bottleneck in the computation time of covariances is the calculation of isotropized power spectra. The current implementation calculates this at each scale factor considered in the computation. By separating the growth factor from this computation, one can save computation time and maintain accuracy to a good approximation.
The implementation includes optional flags when calculating the 2h_22, 3h, and 4h Trispectrum terms in
pk_4pt.py
and the corresponding functions for wrappers.