Compressed waveforms bank workflow #4969
Conversation
tools/install_travis.sh
Outdated
| @@ -50,5 +50,8 @@ pip install -r companion.txt | |||
| echo -e ">> [`date`] installing mpi4py" | |||
There was a problem hiding this comment.
Are we using this script anymore?
There was a problem hiding this comment.
Hmmm - that might be why Im having trouble getting the CI to work! I'll have a look once i am back from holiday
41647e3 to
9989f4d
Compare
|
Note this needs gwastro/sbank#64 I'm not sure whether to temporarily point to my branch, or to wait for a release |
a725f05 to
867d6ff
Compare
867d6ff to
f23706f
Compare
pycbc/waveform/bank.py
Outdated
|
|
||
| try : | ||
| if not (self.has_compressed_waveforms and self.enable_compressed_waveforms): | ||
| raise ValueError |
There was a problem hiding this comment.
I'm a bit concerned that we could have a situation where get_decompressed_waveform isn't working, but we don't know because we're always falling back on the slower method.
There was a problem hiding this comment.
I've added a UserWarning so that if this happens, at least there is evidence, and the message of which includes the approximant as well, so that it is immediately obvious if something is wrong
There was a problem hiding this comment.
(Note that as the message changes, it will not be supressed for different approximants when repeated, see https://docs.python.org/3/library/warnings.html#repeated-warning-suppression-criteria)
There was a problem hiding this comment.
I meant that we should distinguish between self.has_compressed_waveforms being false, where we should silently fall back, and a call to self.get_decompressed_waveform( failing, which would indicate a genuine problem.
There was a problem hiding this comment.
(Not clear why self.get_decompressed_waveform failures would not be ValueErrors???)
There was a problem hiding this comment.
Ah, that makes sense. I've moved to a combined if/else with try/except which doesn't rely on the get_decompressed_waveform function not raising a ValueError
pycbc/workflow/core.py
Outdated
| @@ -1122,12 +1122,13 @@ def __init__(self, ifos, exe_name, segs, file_url=None, | |||
| exe_name: string | |||
| A short description of the executable description, tagging | |||
| only the program that ran this job. | |||
| segs : igwn_segments.segment or igwn_segments.segmentlist | |||
| segs : igwn_segments.segment or igwn_segments.segmentlist or None | |||
There was a problem hiding this comment.
This change is rejected (or at least musn't be bundled in with this patch). Following other workflows, we provide a "dummy" segment if there is not a logical one to give, but a segment should still be given.
There was a problem hiding this comment.
This has been reverted - it means that the files in the workflow are all named "...-0-0.ext" or similar, but that is a minor thing
|
CI failure looks to be a Segfault in the 3-IFO coinc jobs - I will double-check that the inspiral output is the same before and after this change before I merge |
|
@spxiwh - I've just realised that this PR will need cherry-picked onto the v23 release branch so that the mixed-compressed banks (SPATmplt not compressed, SEOB... compressed) will work Sorry for the confusion |
* Add compressed waveforms to bank workflow * Allolw plotting script to use any bank conversion parameter * Some fixes to allow the joined bank to be plotted * Use inference's parameter labels: they are available and mostly good * Add mismatch to plotting, make some tweaks * some tidying * thinko * Try to make the CI workflow run * Fix do-not-compress default * Use different examples in compress bank workflow * Proper name for the github workflow * Thinko * python shebang in compression workflow script * minor edits * move to readily-available waveform * TRy IMRPhenomD instead * revert change to workflow.core * Warn for KeyError in get_decompressed_waveform * Fix issue with if get_decompressed_waveform raised a ValueError
* Add compressed waveforms to bank workflow * Allolw plotting script to use any bank conversion parameter * Some fixes to allow the joined bank to be plotted * Use inference's parameter labels: they are available and mostly good * Add mismatch to plotting, make some tweaks * some tidying * thinko * Try to make the CI workflow run * Fix do-not-compress default * Use different examples in compress bank workflow * Proper name for the github workflow * Thinko * python shebang in compression workflow script * minor edits * move to readily-available waveform * TRy IMRPhenomD instead * revert change to workflow.core * Warn for KeyError in get_decompressed_waveform * Fix issue with if get_decompressed_waveform raised a ValueError
* Merge part of 4997 to avoid failure * Bump to v2310 * Compressed waveforms bank workflow (#4969) * Add compressed waveforms to bank workflow * Allolw plotting script to use any bank conversion parameter * Some fixes to allow the joined bank to be plotted * Use inference's parameter labels: they are available and mostly good * Add mismatch to plotting, make some tweaks * some tidying * thinko * Try to make the CI workflow run * Fix do-not-compress default * Use different examples in compress bank workflow * Proper name for the github workflow * Thinko * python shebang in compression workflow script * minor edits * move to readily-available waveform * TRy IMRPhenomD instead * revert change to workflow.core * Warn for KeyError in get_decompressed_waveform * Fix issue with if get_decompressed_waveform raised a ValueError * Combined plotifar (#5034) * added plot script * cleanups * remove now unused bits * Generalize fit plotting * rename script * Added page_farstat in summary (#5052) * Fix release naming * Stat correction patch (#5061) * Limit number of stage output jobs * Reorganize FAR/stat plots on summary page (#5061) * Get the dq files into a nice layout (#5064) * Sphinx version CI fix (#5060) * Update pycbc_page_fars_vs_stat (#5067) --------- Co-authored-by: Gareth S Cabourn Davies <gareth.cabourndavies@ligo.org> Co-authored-by: Thomas Dent <thomas.dent@usc.es> Co-authored-by: Rahul Dhurkunde <rahul.dhurkunde@ldas-pcdev6.ligo.caltech.edu>
* Add compressed waveforms to bank workflow * Allolw plotting script to use any bank conversion parameter * Some fixes to allow the joined bank to be plotted * Use inference's parameter labels: they are available and mostly good * Add mismatch to plotting, make some tweaks * some tidying * thinko * Try to make the CI workflow run * Fix do-not-compress default * Use different examples in compress bank workflow * Proper name for the github workflow * Thinko * python shebang in compression workflow script * minor edits * move to readily-available waveform * TRy IMRPhenomD instead * revert change to workflow.core * Warn for KeyError in get_decompressed_waveform * Fix issue with if get_decompressed_waveform raised a ValueError
In order to make the generation of the bank with compressed waveforms much easier, and making this more straightforward for future developers/users, I've put this into a workflow.
I've also made changes which mean that when running
pycbc_compress_bank, we can exclude certain waveforms; e.g. SPAtmplt, and these will be ignored when making the compressed bank.When fetching the compressed bank, the fact that the template hash key isn't present will then mean that the waveform is just generated as normal.
Standard information about the request
This is a new feature
This change affects the offline search
This change changes nothing for scientific output, just makes things easier
This change: has appropriate unit tests, follows style guidelines (See e.g. PEP8), has been proposed using the contribution guidelines
This change will require additional dependencies for the CI: sbank
sbank will also need updated from its curent form as it needs a tweak i made to the hdf5 bank combiner there (I will link PR when I have made it)
Contents
I'll split this into subsections:
Minor changes to compression
Making a workflow to generated the compressed waveform bank(s)
So that we can parallelise nicely, I've put this into a workflow which:
The plots are similar to:
the number of / types of these plots can be chosen in the config file
Links to any issues or associated PRs
sbank PR:
Testing performed
The workflow was run, and the results page can be seen at https://ldas-jobs.ligo.caltech.edu/~gareth.cabourndavies/testoutput/compress_bank_workflow/testing/compress_bank_results_page_new/
I will add more tests (e.g. profiling) soon