ENH: Add triangular interferometer

[filippo-santoliquido]

Hi,

I am opening this pull request to enable Dingo to work automatically with a triangular interferometer configuration, such as the Einstein Telescope (ET).

I am aware that the current implementation is not particularly elegant. In its present form, creating a new interferometer object in Bilby also requires modifying the sections of code shown below, which is not ideal. The main issue is that Dingo currently expects data_settings["detectors"] to contain the full list of interferometers, and this list is directly passed to InterferometerList. However, doing so results in an error from Bilby, since the interferometer 'ET-1' does not exist.

This happens because when InterferometerList is called with a triangular configuration (i.e., just 'ET'), it automatically generates all the individual interferometers defined in data_settings["detectors"].

I would appreciate any suggestions for a cleaner or more flexible solution.

Thanks a lot in advance,
Kind regards,
Filippo

[filippo-santoliquido]

Hi,

This PR addresses a limitation in dingo when working with the triangular Einstein Telescope (ET).

Background

Currently, dingo assumes that the detector names in data_settings["detectors"] match exactly the detectors produced by InterferometerList(data_settings["detectors"]). This holds for standard interferometers, but breaks for a triangular ET: when [ET] is passed to bilby's InterferometerList, it automatically expands into three separate detectors.

Proposed fix

The cleanest solution is to decouple the two concepts:

• Pass data_settings["detectors"] (or metadata["train_settings"]["data"]["detectors"]) only to InterferometerList for initialization.
• Retrieve the actual detector names used throughout the code dynamically via ifo_names = [ifo.name for ifo in self.ifo_list], as already done in [injection.py#L310](https://github.com/dingo-gw/dingo/blob/45dbe10982eddc177aa3f5eaf3bf5a4d87145df3/dingo/gw/injection.py#L310).

This PR updates train_builders.py accordingly, building on my previous PR.

Next steps

If this approach looks good, we should audit all places in the codebase where settings["detectors"] (or similar) is accessed directly under the assumption that it reflects the full list of used detectors — and replace those with ifo_names where appropriate.

Happy to discuss further.

Best,
Filippo

[nihargupte-ph]

Hi @filippo-santoliquido, I think this is a good change and makes sense. Currently, I am not approving due to the "Next steps" which I agree with. Is there anywhere else in the code that this should be updated?

[filippo-santoliquido]

Hi @nihargupte-ph, thanks a lot for your reply. I searched for settings["detectors"] across the entire code and found a few additional places where it seems to be used incorrectly. In particular, here:

dingo/compatibility/update_saved_model_new_gnpe.py

Line 32 in 03da18a

f"{ifo}_time_proxy" for ifo in data_settings["detectors"][1:]

, the variable ifo should instead iterate over ifo_names. I also think the usage of ["train_settings"]["data"]["detectors"] should be carefully reviewed and fixed wherever necessary.

[nihargupte-ph]

Hi @nihargupte-ph, thanks a lot for your reply. I searched for settings["detectors"] across the entire code and found a few additional places where it seems to be used incorrectly. In particular, here:

dingo/compatibility/update_saved_model_new_gnpe.py

Line 32 in 03da18a

f"{ifo}_time_proxy" for ifo in data_settings["detectors"][1:]

, the variable ifo should instead iterate over ifo_names. I also think the usage of ["train_settings"]["data"]["detectors"] should be carefully reviewed and fixed wherever necessary.

Sorry for the delay I missed this in my inbox.. Gotcha, I agree with your statement. I think in any case where ifo_names is called one should not iterate over the "train_settings"]["data"]["detectors"] but rather over the ifo_names via [ifo.name for ifo in InterferometerList]

[lorenzopompili00]

The line here: https://github.com/filippo-santoliquido/dingo-ET/blob/feature/add-triangular-interferometer/dingo/gw/inference/gw_samplers.py#L319

should be

ifos=[ifo.name for ifo in InterferometerList(self.detectors)],

(e.g., (ET1, ET2, ET3 rather than 'ET').

[lorenzopompili00]

This line here is also currently failing: https://github.com/filippo-santoliquido/dingo-ET/blob/feature/add-triangular-interferometer/dingo/pipe/main.py#L179

The problem is a naming mismatch: bilby expands ET -> ET1/ET2/ET3, so the ASD dataset is keyed by the three arms, but bilby treats ET as a single detector. So the PSD loop in fill_in_arguments_from_model does asd_dataset.save_psd(..., 'ET') which gives KeyError: 'ET'. We can't just key psd_dict by ET1/ET2/ET3 either, because bilby_pipe's _validate_psd_dict checks against valid Bilby detectors, and only ET is a valid option.

A potential fix is to read one arm's ASD as the representative but keep the psd_dict key as 'ET', e.g.:

for ifo_name in args.detectors:
    if ifo_name in asd_dataset.asds:
        asd_key = ifo_name            # normal detector, e.g. 'H1'
    else:
        asd_key = [ifo.name for ifo in InterferometerList([ifo_name])][0]  # 'ET'-> 'ET1'
    psd_path = asd_dataset.save_psd(args.outdir, asd_key, rng=rng)
    psd_dict[ifo_name] = str(psd_path)  # keyed by 'ET'

One thing to confirm: this saves a single PSD, assuming it is the same across the three arms. That is consistent with how ET is currently defined in bilby (all three arms use the same PSD), so I think it's probably fine, do you think this makes sense @filippo-santoliquido? If we want independent per-arm PSDs it won't be enough however (though that likely needs a fix in bilby/bilby_pipe as well to understand ET1/ET2/ET3 as separate detectors).

[lorenzopompili00]

Thank you @filippo-santoliquido!

I have tried running a quick end-to-end test for an injection based on #386 (changing detectors = [H1, L1] to detectors = [ET]) and with the fixes I commented above it managed to run successfully.

[filippo-santoliquido]

Hi @lorenzopompili00! Thanks a lot for your work! I believe that in the future we'll need to implement ET with different PSDs for each detector, to make our simulations as close to reality as possible. In any case, we'll need to fix bilby first, as you said, and this fix will then be automatically inherited by dingo.

[filippo-santoliquido]

Dear @lorenzopompili00 ! Thanks a lot again for your thorough review. I should have implemented all your comments.