Modify injection module to support space-based detector class #4982
Conversation
acorreia61201
force-pushed
the
lisa-emri-pe-injection
branch
from
493b888 to
9354cce
Compare
| @@ -1,2 +1,4 @@ | |||
| from .ground import * | |||
| from .ground import _ground_detectors | |||
There was a problem hiding this comment.
If we want to import these, they shouldn't have a _ prefix. The prefix is to indicate that the initial intention was only use within the module and through functions in that module. I would check if you really need to access this or if you can use a method of the Detector class directly.
| elif detector_name in get_available_space_detectors(): | ||
| au_travel_time = lal.AU_SI / lal.C_SI | ||
| try: | ||
| arm_travel_time = _space_detectors[detector_name]['armlength'] / lal.C_SI |
There was a problem hiding this comment.
This is where you'll need to think about what the time coordinate means in the injection file. Is it in the SSB time or the detector frame? If the SSB frame, this isn't the right time to consider. If the detector frame, it is close, but do we want to rely on the detector geometry having a single fixed arm length argument (this is a candidate for the detector class providing this information if it is really needed).
There was a problem hiding this comment.
I guess the choice here is safe though, assuming the orbit is at an AU (probably ok?).
| """ | ||
| # the start times are the tcs | ||
| tcs = self.table.tc | ||
| # FIXME: this could be figured out using the ringdown module | ||
| return tcs + 2 | ||
| mfs = self.table.final_mass |
There was a problem hiding this comment.
Does this belong in this PR? Or should this be a small separate one that can be reviewed quickly?
| # try converting to a list of strings; this function will just | ||
| # return val if it does not begin (end) with [ (]) | ||
| static_args[key] = _convert_liststring_to_list(val) | ||
| if val is None or val == '': |
There was a problem hiding this comment.
Does this belong in this PR or a separate one?
| @@ -67,12 +67,13 @@ def apply_polarization(hp, hc, polarization): | |||
|
|
|||
| return hp_ssb, hc_ssb | |||
|
|
|||
| def check_signal_times(hp, hc, orbit_start_time, orbit_end_time, | |||
| offset=TIME_OFFSET_20_DEGREES, pad_data=False, t0=1e4): | |||
| def preprocess(hp, hc, orbit_start_time, orbit_end_time, | |||
There was a problem hiding this comment.
I think you may need to rebase your PR from master.
| @@ -638,8 +672,13 @@ def make_strain_from_inj_object(self, inj, delta_t, detector_name, | |||
| # compute the waveform time series | |||
| hp, hc = get_td_waveform(inj, delta_t=delta_t, f_lower=f_l, | |||
| **self.extra_args) | |||
| strain = projector(detector_name, | |||
| inj, hp, hc, distance_scale=distance_scale) | |||
| if detector_name in get_available_space_detectors(): | |||
There was a problem hiding this comment.
Can we make the output uniform here so that we don't need the if statement?
This PR adds support for generating injections using a space-based detector response.
This change affects: injection generation.
This change changes: primarily the
injectmodule, with some minor additions to thedetectoranddistributionsmodules to be compatible with theinjectchanges. The latter changes should be purely additive, though; the behaviors of ground-based detectors should remain unchanged from current usage.Motivation
A method for generating and reading frame files for space-based signals is required for eventual space-based PE. While #4691 adds the ability to generate space-based detector responses by-hand, the changes here allow for this to be written to a frame file via the
injectmodule. Similar functionality is available via BBHx for MBHBs. However, this PR seeks to add support for general space-based signals natively in PyCBC, ideally matching the current output of BBHx for the same injection parameters.Contents
injectmodule to apply simulated signals to a strain using a space-based detector response.project_wavemethod in detector classes to accept kwargs from injection filedetectorclasses that print off accepted sky location parameters (e.g. ra/dec, ecliptic longitude/latitude)distributionsLinks to any issues or associated PRs
The space-based response is applied using the code changes from PR #4691. Any changes to that PR will be applied here once that PR is merged to master.