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.

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

[BUG] Convert LALDict to python dictionary before saving #825

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

adivijaykumar
Copy link
Collaborator

Fixes #751. Uses lalsimulation.gwsignal.core.utils.from_lal_dict to convert a LALDict to a python dictionary before saving.
I have a few concerns which I was not sure how to tackle:

The new logic has the downside that it is the only part of code in core/result.py that depends on a function in gw. The other option is to define an equivalent _get_save_data_dictionary inside the CompactBinaryCoalescenceResult class.
Right now this imports the relevant function from LALSimulation, but we could have just copied over the relevant code into bilby (since LALSuite is under GNU GPL).

Migrated from git.ligo: https://git.ligo.org/lscsoft/bilby/-/merge_requests/1309

@adivijaykumar
Copy link
Collaborator Author

Comment from Colm on the original MR:

I think it would be neater to have this in this class https://git.ligo.org/lscsoft/bilby/-/blob/master/bilby/core/utils/io.py#L29 and a coresponding deserialiser in https://git.ligo.org/lscsoft/bilby/-/blob/master/bilby/core/utils/io.py#L148.
We may also need a pair for hdf5 (https://git.ligo.org/lscsoft/bilby/-/blob/master/bilby/core/utils/io.py#L209, https://git.ligo.org/lscsoft/bilby/-/blob/master/bilby/core/utils/io.py#L247). The main issue is that we should not add an import of lalsimulation at the top of that method. For the encoder I'd suggest something like we do for e.g., pandas objects

@mj-will mj-will added the bug Something isn't working label Oct 7, 2024
@adivijaykumar adivijaykumar marked this pull request as draft October 11, 2024 07:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working gitlab merge request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Saving result file after sampling fails if lal dict is passed
2 participants