Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add any provided key to lal_pars for waveform generation #4728

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Add any provided key to lal_pars for waveform generation #4728

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f126512
add functionality to add any provided key to the LALDict
hoyc1 May 1, 2024
e4b7d97
typo
hoyc1 May 1, 2024
2f2ca04
typo
hoyc1 May 1, 2024
e050d04
only add non-default parameters to lal_dict
hoyc1 May 1, 2024
42b6a66
typo
hoyc1 May 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions pycbc/waveform/waveform.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,21 @@ def _check_lal_pars(p):
The lal type dictionary to pass to the lalsimulation waveform functions.
"""
lal_pars = lal.CreateDict()
#nonGRparams can be straightforwardly added if needed, however they have to
# be invoked one by one
# try and add all provided items to lal_pars. We try both the provided key
# as well as a camel case version
for key, value in p.items():
if (value is None) or (key in default_args.keys()):
continue
camelcase = "".join([subkey.capitalize() for subkey in key.split("_")])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can any of the following lines be removed then if this is added? It would seem to serve in large part a similar purpose. Not all of course (modes for example would not work), but some of the nongr params probably could be not explicitly given now.

Copy link
Contributor Author

@hoyc1 hoyc1 May 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ahnitz, I would need to change the logic to account for the nongr params, or potentially deprecate the existing pycbc names and change them to new names. For example, the current nongr params in pycbc follow the convention e.g. dchi0 but the lalsimulation function requires them to be NonGRDChi0. I could remove frame_axis and modes_choice, but they would also need to be removed from the default_args (in order to prevent the issue above).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I don't think we want to change the standardized names. One could remove code duplication with a dict, though, no?

Also, have you checked how slow this procedure is? What sort of overhead does it introduce?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, the LALDict internally uses exactly the same field name as pycbc anyway (https://git.ligo.org/lscsoft/lalsuite/-/blob/master/lalsimulation/lib/LALSimInspiralWaveformParams.c#L1326) - it is only the name of the 'Insert' function that is unpredictable.

After importing lalsimulation it is possible to get a mapping from the dictionary key to the Insert function in any case - LVK people can see here how to do that via dir(lalsimulation) (it's not anything that should be secret ...)
https://git.ligo.org/waveforms/new-waveforms-interface/-/issues/49#note_1028009

for _key in [key, camelcase]:
_func = getattr(
lalsimulation, f"SimInspiralWaveformParamsInsert{_key}", None
)
if _func is not None:
_func(lal_pars, value)
break
# some keys require specific functions to be added to lal_pars. Below we
# add these parameters one by one
if p['phase_order']!=-1:
lalsimulation.SimInspiralWaveformParamsInsertPNPhaseOrder(lal_pars,int(p['phase_order']))
if p['amplitude_order']!=-1:
Expand Down Expand Up @@ -133,6 +146,7 @@ def _check_lal_pars(p):
for l,m in p['mode_array']:
lalsimulation.SimInspiralModeArrayActivateMode(ma, l, m)
lalsimulation.SimInspiralWaveformParamsInsertModeArray(lal_pars, ma)

#TestingGR parameters:
if p['dchi0'] is not None:
lalsimulation.SimInspiralWaveformParamsInsertNonGRDChi0(lal_pars,p['dchi0'])
Expand Down
Loading