CompactBinaryCoalescenceResult methods returning different objects

[foocheng]

This issue is relevant to bilby.gw.result.CompactBinaryCoalescenceResult. It seems that calling CompactBinaryCoalescenceResult.from_json("...") returns the correct bilby.gw.result.CompactBinaryCoalescenceResult object, while calling CompactBinaryCoalescenceResult.from_hdf5("...") returns only a bilby.core.result.Result object, meaning the resulting object will not have the class properties of the full gw.result.CBCResult class.

[ColmTalbot]

Thanks for the report @foocheng are you able to provide a working example to help with debugging?

[foocheng]

Hi @ColmTalbot, after some digging it looks like the issue is with accessing parallel_bilby results with the CBCResult class. I realise this might be an issue to potentially open on the parallel_bilby channel instead -- regardless, for completion here is the explanation and a minimal working example.

Inside bilby.core.result.Result.from_hdf5, the function looks for data['__module__'] and data['__name__'] to figure out class type to call. It seems like there is an issue in the parallel_bilby result generation that means print(data['__module__'], data['__name__']) gives bilby.core.result Result instead of bilby.gw.result CompactBinaryCoalescenceResult, as is the case with result files generated with bilby_pipe. So even if I call the result file using CBCResult.from_hdf5, the data in the result files overwrites the class I am trying to call.

Minimal Working Example:

Creating a parallel_bilby generated result file (similar to this minimal example for bilby_pipe, but for parallel_bilby).
Rename this tobbh_injection_parallel.ini
bbh_injection_parallel.txt
Rename this to bbh_injection_prior.prior
bbh_injection_prior.txt

Run the following command to create injection data
bilby_pipe_create_injection_file bbh_injection_prior.prior --n-injection 10 --generation-seed 1234 -f injections.json

Create result file with parallel_bilby:
parallel_bilby_generation bbh_injection_parallel.ini --submit

Then, when trying to call the results,
from bilby.gw.result import CBCResult
CBCResult.from_hdf5("outdir_bbh_injection/result/bbh_injection_0_result.hdf5") returns a <bilby.core.result.Result> object, without any of the CBCResult functionality, while
CBCResult.from_json("outdir_bbh_injection/result/bbh_injection_0_result.json") returns a <bilby.gw.result.CompactBinaryCoalescenceResult> result with the correct CBCResult functionality.

[ColmTalbot]

Thanks, this is very helpful! I agree there are two issues:

• parallel_bilby writes a Result object where it should really write a CBCResult (https://git.ligo.org/lscsoft/parallel_bilby/-/blob/master/parallel_bilby/analysis/read_write.py?ref_type=heads#L190), I think this is outside the scope of this package. If you can open an issue or MR into parallel bilby, that would be good, but I don't think that package is actively maintained.
• Bilby is inconsistent between what class is returned for the two read methods. This one we should reconcile, the question is whether we should take as precedence the type of the result saved, or the the type being used to read the file, or some hybrid where only the base class is updated. I think we should discuss this on an upcoming development call.

[mj-will]

Bilby is inconsistent between what class is returned for the two read methods. This one we should reconcile, the question is whether we should take as precedence the type of the result saved, or the the type being used to read the file, or some hybrid where only the base class is updated. I think we should discuss this on an upcoming development call.

I think this might be related to this logic. From a quick test, it seems that when loading an HDF5 file, it fails to correctly fetch the class and defaults to the normal Result class though the warning there doesn't seem to be printed for some reason (I do get a warning about the priors though).

From looking at the code, I also think the type of result saved is meant to take priority at the moment (and does for the JSON version).

[mj-will]

We should add some tests to ensure everything is consistent and the correct class is being loaded.