State-of-the-art of timeslides development in pycbc_multi_inspiral #4177
Conversation
There was a problem hiding this comment.
This is confined to multi_inspiral things only, so I'm happy as long as it passes the various tests. I did leave a few comments in the module code to request some clarity. I didn't look at the front end, but have no issue with the approach you describe of bringing developing onto the main branch, as long as nothing other than multi_inspiral is affected by it.
| @@ -80,7 +80,9 @@ def get_coinc_triggers(snrs, idx, t_delay_idx): | |||
| coincs: dict | |||
| Dictionary of coincident trigger SNRs in each detector | |||
| """ | |||
| coincs = {ifo: snrs[ifo][idx + t_delay_idx[ifo]] for ifo in snrs} | |||
| coincs = { | |||
| ifo: snrs[ifo][(idx + t_delay_idx[ifo]) % len(snrs[ifo])] | |||
There was a problem hiding this comment.
I'm not quite sure what snrs[ifo] is actually storing at this point .... or is the problem that idx + t_delay_idx[ifo] might wrap around the start (or the end)? We do want to keep track of this, so that any quoted delay times in the output file correspond to physical shift (so if we are at t=1, and shift by -100, we give the time-slide as len - 100, not as -100.)
| if window_size == 0: | ||
| indices = numpy.arange(len(tvec)) | ||
| else: | ||
| if not window_size == 0: |
There was a problem hiding this comment.
This function could definitely do with some comments ... We don't want to rewrite the code and end up with something as opaque as coh_PTF is :-)
|
@pannarale Any update on this one? |
Adding some comments to timeslides. Additional info should be added.
Removed whitespace and made small edits to comments.
Removed a whitespace
Fixing whitespace issues and a line that was too long
|
@spxiwh, @ETVincent made a few changes? Are you happy to squash and merge? |
…wastro#4177) * Uploading state-of-the-art timeslides work for further development * non verbose option for wget * Comment additions Adding some comments to timeslides. Additional info should be added. * Update to comments Removed whitespace and made small edits to comments. * More comment updates Removed a whitespace * Updates to comments Fixing whitespace issues and a line that was too long Co-authored-by: ETVincent <98178306+ETVincent@users.noreply.github.com>
…wastro#4177) * Uploading state-of-the-art timeslides work for further development * non verbose option for wget * Comment additions Adding some comments to timeslides. Additional info should be added. * Update to comments Removed whitespace and made small edits to comments. * More comment updates Removed a whitespace * Updates to comments Fixing whitespace issues and a line that was too long Co-authored-by: ETVincent <98178306+ETVincent@users.noreply.github.com>
This update is to be considered WIP, as is
pycbc_multi_inspiralafter all. The idea is to bring to master work done by other developers who no longer work on this project, so that current developers can complete it before things get too outdated.pycbc_multi_inspiralis not used to the best of my knowledge, aside from the mentioned ongoing development.The PR comes with its single
pycbc_multi_inspiraltest with one sky point and one timeslide.