Need to be more careful in truncate_waveform_by_flow

[vijayvarma392]

https://github.com/vijayvarma392/gw_eccentricity/blob/e1e3a1dc11ecf8afac105db205bbf6cafa8fa11a/gw_eccentricity/truncate_waveform_by_flow.py#L77

Does this code first compute pericenters when getting gwecc_object and then recompute them again? That's very bad! Similarly, why repeat the code about retaining good extrema, and building the interpolant? If that's not what is happening: It's still bad to repeat this whole section of code. How about making a function in the main code and just calling that in both places. This is important because, let's say you decide to change the 1,5 factor in one place and forget to change it in the other one..

Finally, independent: What does this code do if you give it a waveform that is too short for flow=20Hz, but ask it to truncate it at flow=20Hz? It should raise a helpful error message.

[vijayvarma392]

@md-arif-shaikh: You said something about the first part above, but I lost it. Can you say it again? My main objection is to repeating the code to drop extrema, espeically factors like 1.5 that might change in the main code later on. Is there a way to avoid that? For example, what if get_good_extrema is made smart enough to recognize when apocenters is None?

What about the second part above?

[vijayvarma392]

With some more hindsight, I'm starting to think maybe we just want this code to be a part of eccDefinition itself. What do you think?

The main reason would be to make it not as hidden as it is now. And the interface would be more obvious. You always start with:

gwecc_object = available_methods[method](
            dataDict, num_orbits_to_exclude_before_merger, extra_kwargs)  

Then for the full thing you do:

        tref_or_fref_out, ecc_ref, mean_ano_ref = gwecc_object.measure_ecc(
            tref_in=tref_in, fref_in=fref_in) 

For truncation, you'd do:

dataDict = gwecc_object.truncate_at_flow(flow=flow, m_max=m_max)

Finally, we'd want a wrapper similar to measure_eccentricity in gw_eccentricity.py.

If you agree, I'd just take a note of this and do this after the paper.

[md-arif-shaikh]

I am leaving this for later. I will get back to it after the paper is done.

[vijayvarma392]

Ok, among the current issues, I think this is among the high-priority ones, but as time permits. The reason is this would be an interface change, and it's good to not do those after making the code public.