Fix the growth function g in growth_rate() and growth_rate_fn()#197
Fix the growth function g in growth_rate() and growth_rate_fn()#197MinhMPA wants to merge 3 commits intohalomod:mainfrom
Conversation
steven-murray
left a comment
steven-murray
left a comment
There was a problem hiding this comment.
Thank you @MinhMPA!
I think this is good, but I think it needs to more things:
- Since this is technically API-breaking (some folks may have been relying on the
growth_ratebeing defined this way) I'd prefer to actually add a warning to the function temporarily to tell users that it has changed. - There is one breaking test, which I think comes from the fact that the
GenMF.growth_ratefunction also needs to be updated to have this definition.
| def growth_rate(self, z): | ||
| """ | ||
| Growth rate, dln(d)/dln(a) from Hamilton 2000 eq. 4 | ||
| Note that the growth function g in Hamilton 2000 is defined as g=D*(1+z). |
There was a problem hiding this comment.
While we're at it, can we update the docs to point to the arxiv or DOI of the paper? (https://arxiv.org/pdf/astro-ph/0006089.pdf)
There was a problem hiding this comment.
Thanks for the feedback! You meant implement:
- A similar fix to the growth rate method in
GenMF? And - A python built-in
UserWarningfor each method?
I can do so, but maybe we should just merge this with the fix to the issue I mentioned in #198?
There was a problem hiding this comment.
I think that's a good idea -- it might be best to figure out what's going on in #198 and add the fix to this PR. I think the fix for GenMF should not be difficult, and neither should the warning (perhaps you can add it to the base class). I'm still a little hesitant on the growth rate differences between CAMB and the numerical integral. I'd like to understand it better, but I have no time in the next couple of weeks.
2 participants
PR to address issue #196 .