Align Einstein Telescope geometry with LAL

[milanwils]

Problem
As mentioned in #713, the current geometry of the ET is not in line with its definition in LAL.

1. The labels of ET2 and ET3 are swapped.
2. Currently the new corner points are calculated by updating the latitude and longitude approximately and not updating the elevation. Additionally, the azimuth and tilt do not take into account the curvature of the earth. This leads to small differences in the vertex position and unit vector. As a result, the null stream is not accurate and signal leaks into it.

Proposed Solution

1. This is addressed in this PR by adding an optional clockwise option to the detector config. I set the value in the ET.interferometer file to clockwise=False so that the default is consistent with LAL. If this is undesired for backwards compatibility, it could be set to clockwise=True .
2. This issue is addressed by following the same specification as LAL: T1400308. The code is rather verbose to avoid confusion but can be changed if denser code is preferred. This algorithm required the implementation of a transform from WGS84 coordinated to ECEF coordinates. I added a small test to check the consistency with the forward transformation. In the implementation of this transform I used uninformative variable names to be consistent with the reference (they do not have a physical meaning anyway).

Additional information
There is also a small error in the LAL config, see LAL PR. Only if both get accepted do the configurations become identical.

[ColmTalbot]

This is a nice change, I think we'll need to think a little about the best way to do this. I've suggested waiting a while to get this in as I don't think it is urgent, the labelling of ET2/ET3 doesn't really matter, and we can make the changes more satisfyingly if we break things.

[ColmTalbot]

I know this is not the change for the current code, but given that this is clearly better thought out than the current method, can the logic for updating the geometry be moved into a standalone function (with a nice docstring)?

[milanwils]

Perhaps we could add setters to the InterferometerGeometry class for x, y and vertex? For the lat, long, elevation, tilt and azimuth these setters are separate but it does not seem sensible to me that someone can change only one Cartesian coordinate.

All the update logic could then be moved into the geometry class and only the calculation of the new corner points (which is triangle specific) remains in the 'TriangularInterferometer` class.

My main concern with this is approach is that one would need to be careful to update the vertex position before changing the unit vector. Otherwise the calculation of tilt and azimuth is wrong. This could be avoided by making the unit vectors the "base information" and deriving the tilt and azimuth from it.

This also raises the question of whether the constructor should be adapted to allow users to define the detector directly from Cartesian parameters. I guess you could argue that this is actually the only way you should be able to set these parameters as interferometers are constant.

[ColmTalbot]

I would personally vote for just making a breaking change here (and waiting for a major release) to avoid adding a new parameter that doesn't really mean anything (the set of three Interferometers are just a permutation of each other with the different directions).

[milanwils]

Fine by me, if people really want they can still override the detector names

[ColmTalbot]

I feel like this function should take and recieve the fame type, either (float, float, float) -> (float, float, float) or array -> array. I don't really mind which, but it would be good to be consistent.

I now see that the same thing happens in get_vertex_position_geocentric. sigh, I'm not sure if it is worth changing the other function.

We could change the other function to something like

def get_vertex_position_geocentric(position, _longitude=None, _elevation=None):
    if longitude is not None and not isarray(position):
        WARN("... syntax changed in Bilby 3.0.0 ...")
        position = np.array([position, _longitude, _elevation])
    ...

This would need a wider set of eyes/opinions.

[milanwils]

I went through the exact same thought process when writing the function. This seems like a good compromise.

[milanwils]

Update: LAL MR2399 has been approved and the methodology in T1400308 now contains the algorithm added in this PR.

[milanwils]

For future reference: the transformation is also implemented in astropy (this is what pycbc uses).