Imr phenom xhm support

[sclyne623]

adds the IMRPhenomXHM approximant to the list of valid approximants in the generate_FD_modes_LO function and added a conditional check before convert_J_to_L0_frame as IMRPhenomXHM is not generated in the J frame.

[mpuerrer]

It would be good to add a comment which explains why IMRPhenomXHM behaves differently from IMRPhenomXPHM. In the code we see that both approximants are supported in the if statement in line 664, but in line 679 the check is only for IMRPhenomXPHM. Don't leave the reader guessing, but give a clear and concise statement about why they need to be treated differently.

[mpuerrer]

You deleted the comment about "approximants that are implemented in J frame"; rather than just deleting it would be better to explain that some waveforms are in the J frame, but others are not, and listing the ones considered here.

[mpuerrer]

Thanks for updating line 661. If you read through the comments in the entire function, you'll see that there are other places where the comments should be updated: in line 674 it would be good to specify that "For some waveform models considered here ... the modes are returned in the J-frame".

And it would be good to add near the beginning of the function -- line 662 which frame each of the waveform model's modes are give in, perhaps like this:

# IMRPhenomXPHM: modes in J-frame
# IMRPhenomXHM: modes in L-frame

It also isn't necessary to list the numbers 101, 97 anymore because we have the names and the numbers come from a C enum in LALSimulation which would change if someone were to insert a waveform before a particular approximant.

[mpuerrer]

Thanks, this looks good to me.

[mpuerrer]

@stephengreen please have a look as well if you are happy with this change.

[stephengreen]

Thanks @sclyne623. To be sure that this works properly, please extend the test

dingo/tests/gw/waveform_generator/test_wfg_m.py

Line 142 in 7fd8ac8

def test_generate_hplus_hcross_m(intrinsic_prior, wfg, num_evaluations, tolerances):

to include IMRPhenomXHM.