dingo-gw / dingo

Dingo: Deep inference for gravitational-wave observations
MIT License
51 stars 16 forks source link

adding new conditioning for td modes #217

Closed hectorestelles closed 9 months ago

hectorestelles commented 10 months ago

This PR adds a conditioning method for TD modes mimicking the standard behaviour of conditioning routines in LAL and GWSignal, i.e, starting the modes at a lower frequency for tapering not-in-band cycles and adding some extra buffer times.

stephengreen commented 10 months ago

I improved the mode reconstruction tests, including now a test for SEOBNRv5HM. When I ran the test on SEOBNRv5HM 1,000 times, I found mismatches up to 0.1. For SEOBNRv5PHM they stayed below 1e-7. Generally, mismatches are worse for HM than PHM.

stephengreen commented 10 months ago

Also, since we now have a unit test for v5 waveforms, do we need to adjust the dependencies of Dingo, e.g., minimum version numbers to have v5 included?

hectorestelles commented 9 months ago

Hi @stephengreen I tried to address the comments on commit https://github.com/dingo-gw/dingo/pull/217/commits/18c9b30ff77dd9da0bf170d95230dedc907893f2

Let me know if you have further suggestions

hectorestelles commented 9 months ago

Looks good. I updated the test tolerances and added a comment on the unit conversions.

Do we need to specify a minimum lalsimulation version for this test to pass?

Yes, we would need to request lalsimulation>=7.15, in order to have gwsignal on it.