Add support for two new search parameters eccentricity and rel_anomaly in the postprocessing stage to display them#5275
Add support for two new search parameters eccentricity and rel_anomaly in the postprocessing stage to display them#5275yi-fan-wang wants to merge 10 commits intogwastro:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds support for two new search parameters, eccentricity and rel_anomaly, required by the SEOBNRv5E(HM) waveform approximant. These parameters are integrated into postprocessing displays and trigger analysis workflows.
Changes:
- Added optional eccentricity and rel_anomaly parameters to template extraction and waveform plotting functions
- Added property accessors for eccentricity and rel_anomaly in the HDF interface
- Extended parameter fitting support to include mass ratio (q) and eccentricity
Reviewed changes
Copilot reviewed 3 out of 10 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pycbc/workflow/minifollowups.py | Adds eccentricity/rel_anomaly parameter extraction and command-line options for waveform generation |
| pycbc/io/hdf.py | Adds property accessors for eccentricity and rel_anomaly from HDF template banks |
| pycbc/events/triggers.py | Extends parameter extraction to support mass ratio (q) and eccentricity in trigger fitting |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # display eccentricity and relative anomaly if they exist | ||
| try: | ||
| data[-1].append('%5.2f' % bank['eccentricity'][tid]) | ||
| data[-1].append('%5.2f' % bank['rel_anomaly'][tid]) |
There was a problem hiding this comment.
Different models uses different names here. Any thoughts on how to generalize? Or simply report everything that's in the file?
There was a problem hiding this comment.
I think this would be more relevant when we have more than one eccentric waveform models in a single injection set/template bank. Displaying all the parameters probably mean there would be more than one anomaly parameters, while only one of them take effects a time for a specific waveform model. I don't think it's ideal.
I prefer to set up a unified naming convention for anomaly and eccentricity from the PyCBC side, then map them to the specific names of each waveform. This can be done in a waveform plugin which ideally is able to call all eccentric waveform models. But the downside would be a parameter would mean different things in different models (not necessary a downside because that's just the fact?)
Standard information about the request
This update adds support for two new search parameters, "eccentricity" and "rel_anomaly", which are utilized by the SEOBNRv5E(HM) waveform approximant. These parameters have been integrated into all relevant postprocessing displays, including single triggers, background coincidences, foreground coincidences, and injections.