-
Notifications
You must be signed in to change notification settings - Fork 60
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
mocks from galaxy power spectrum for a non-gaussian field #662
base: master
Are you sure you want to change the base?
Conversation
nbodykit/cosmology/power/galaxy.py
Outdated
the type of transfer function used | ||
""" | ||
|
||
def __init__(self, cosmo, redshift ,b0 ,fNL ,p ,Omega_m ,H0=73.8 ,c=3e5 ,transfer='CLASS'): |
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.
Why not using cosmo.h and cosmo.Omega_m?
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.
Oh I completely missed that. That would be so much better. Thanks. Any other recommendations?
nbodykit/cosmology/power/galaxy.py
Outdated
from ..cosmology import Cosmology | ||
from .linear import LinearPower | ||
|
||
class GalaxyPower(object): |
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.
Perhaps rename this to FNLGalaxyPower?
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.
Is there a paper that we can reference for the formula used here? Perhaps we can name the class after that paper?
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.
Hi, here is a paper that has all the formulas and explanations.
nbodykit/cosmology/power/galaxy.py
Outdated
b0 : float | ||
the linear bias of the galaxy in a gaussian field | ||
fnl : float | ||
the non-gaussian parameter |
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.
"Primordial non-gaussian parameter"?
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.
Thanks. I have made all those changes and updated the repo. I will add the unit test part soon. Thanks again.
the linear bias of the galaxy in a gaussian field | ||
fnl : float | ||
the non-gaussian parameter | ||
p : float |
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.
Does this parameter have a more descriptive name from the literature? It is like a bias correction parameter (removed from b)? larger p -> lower clustering?
What is a recently merged halo? A halo that recently experienced a merger event from progenitors of similar masses?
nbodykit/cosmology/power/galaxy.py
Outdated
self.redshift = redshift | ||
|
||
|
||
def corrected_bias(self, k): |
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.
Does the literature call this a "corrected bias"? The return value is called a 'total_bias'. This is a bit confusing...
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.
Both are right but yes, it was confusing to use 2 different terms so I only used "total bias". Some works also refer to it as the "non-gaussian bias" or "fnl-bias". I am not sure which one would be ideal to use here. Let me know your thoughts on this. Thanks.
nbodykit/cosmology/power/__init__.py
Outdated
@@ -1,4 +1,5 @@ | |||
from .galaxy import GalaxyPower |
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.
Let's also add a unit test or two that exercises this file,, e.g., in this file: https://github.com/bccp/nbodykit/blob/a387cf429d8cb4a07bb19e3b4325ffdf279a131e/nbodykit/cosmology/tests/test_power.py
Hi. I have made all the changes and added the unit test functions. Please take a look and let me know. |
Hi, Did you get a chance to take a look into the pull request? |
in response to issue #660