Add BNS catalog - #468
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #468 +/- ##
=======================================
Coverage 98.61% 98.62%
=======================================
Files 114 115 +1
Lines 8819 8849 +30
=======================================
+ Hits 8697 8727 +30
Misses 122 122
🚀 New features to boost your workflow:
|
|
I commented out point_source_type for now because including it in the catalog causes a duplicate input when Source is initialized, since it is already passed through PointSources. I also left the kwargs_kilonova part commented out because these parameters are already passed together through kwargs_kilonova in PointSources, and I’m not sure whether they also need to be stored in the BNS catalog. If not, I can remove these commented lines in the next update. |
|
|
||
|
|
||
| class BNSCatalog: | ||
| """Class to generate a catalog of BNS events within a selected sky area.""" |
There was a problem hiding this comment.
can you write out in the documentation what BNS means?
| self.lightcurve_time = np.linspace(0.1, 10, 50) | ||
|
|
||
| self.kwargs_kilonova = { | ||
| "mej_1": 0.01, |
There was a problem hiding this comment.
can these default values be generated as part of the BNSCatalog class
There was a problem hiding this comment.
document where these numbers are coming from
| :type band_list: list or str | ||
| :param lightcurve_time: Observation time array for the light curve in [days] | ||
| :type lightcurve_time: array-like | ||
| :param kwargs_kilonova: Keyword arguments passed to the Kilonova class. This may |
There was a problem hiding this comment.
can you refer to a redback model for where these arguments are passed in
| :type time_interval: `~Astropy.units.Quantity` | ||
| :param noise: poisson-sample the number of event in the event density lightcone | ||
| :type noise: bool | ||
| :param model_name: The kilonova light curve model to be used. If not provided, |
There was a problem hiding this comment.
can you provide a redback module which this model convention needs to be matched?
|
For the recommended ranges and choices of parameters such as mej_1, vej_1, kappa_1, etc., including some guidance on how the parameters may differ between components, I’m planning to document them in the notebook. If you think this information should also be included in the BNSCatalog documentation, I can add it there and update the PR again. |
sibirrer
left a comment
There was a problem hiding this comment.
Looks great! I have one very minor request but otherwise looks great!
| :type time_interval: `~Astropy.units.Quantity` | ||
| :param noise: poisson-sample the number of event in the event density lightcone | ||
| :type noise: bool | ||
| :param model_name: The kilonova light curve model to be used. The |
There was a problem hiding this comment.
can you also then mention here that the kwargs_kilonova has to match then for this model?
This PR adds a
BNSCatalogclass that usesEventLightconeto generate a BNS source catalog. It also adds tests to check the generated catalog and verify that it can be passed throughPointSourcesto initialize a kilonova source.