added P&R2017 neutral hydrogen mass model #347
Conversation
|
@rrjbca hmm... I'm not sure I agree. HI would be an observable which is fundamentally and conceptually different to the other types of observables we have (galaxies, gravy waves etc) and on a level with them. We could say that 'halos' implies 'Dark Matter halos'. This model also makes a prediction only for HI mass. By your argument, could we also, say, not suggest that the gravitational wave merger rates should also have been placed in galaxies? I can imagine there are things where what you say does make sense though. e.g. multi-wavelength observations of a cluster's gas (HI, X-ray) might require some 'gas' structure within the halos module. |
|
I have had similar thoughts, as the HI models I work with a based on galaxies, so. for this case then, HI model could equally fit into the galaxies module. I think, based on the fact that HI can based on halo and galaxy properties (or possibly other observables), it would make most sense to have it as an own module which can host a variety of HI models. |
|
I meant to say that |
|
Following a discussion we decided that |
itrharrison
changed the title
|
|
||
| Merger Rates |
There was a problem hiding this comment.
this is still the heading from GW, plus too many underlines
| :nosignatures: | ||
| :toctree: ../api/ | ||
|
|
||
| pr_halo_model |
There was a problem hiding this comment.
I don't think this is the best name for the model, mostly because it makes me think that this is a halo model. Suggestions would be hi_mass_pr17 or padmanabhan_refregier_17 (which not everyone likes).
|
|
||
| """ | ||
|
|
||
| from .mass import * # noqa F401,F403 |
There was a problem hiding this comment.
It's ambiguous how the module is to be structured. In this way, you could access the function under both skypy.neutral_hydrogen as well as skypy.neutral_hydrogen.mass. If it's supposed to be the former, then I would add an underscore to _mass to indicate it is an implementation file. If the latter, then you should only from . import mass. (I guess it depends on how much stuff there is going to be in the module to decide which is right.)
| cosmology : `~astropy.cosmology.Cosmology` | ||
| Cosmology object providing values of omega_baryon and omega_matter. |
There was a problem hiding this comment.
This would be better moved to the end of the parameter list because it will be filled by the pipeline runner automatically.
| pr_data = np.asarray([0.00000000e+00, 8.08547768e-45, 6.40736312e+09, 3.66714636e+09, | ||
| 0.00000000e+00, 0.00000000e+00, 0.00000000e+00, 0.00000000e+00]) * units.Msun |
There was a problem hiding this comment.
Is it useful to have half of the tests return zero masses?
Description
Merging will close #234. This implements the Padmanabhan & Refregier HI halo mass model described in that issue.
Checklist