Inconsistency in cartesian to geographic conversion of flow field

[jwookey]

There appears to be an inconsistency in the way that the routines in flow_conversion.py (and test) are described and what they calculate. The cartesian reference frame is described (in geographic.py) as:

The Cartesian system is defined as: x passes through longitude = 0 and latitude = 0 y passes through longitude = 90° and latitude = 0 z passes through the north pole.

The conversion routine output vector is described as [lat, lon, radial], implying that an eastwards component of flow should have a positive second element. This is also implied in test_flow_conversion.py:

def test_west_at_equator(self):
    lat = 0
    lon = 0
    rotated_vector = rotate_vector(self.y_vector, lat, lon)
    expected_vector = np.array([0, -1, 0])
...

def test_east_at_equator(self):
    lat = 0
    lon = 0
    rotated_vector = rotate_vector(self.minus_y_vector, lat, lon)
    expected_vector = np.array([0, 1, 0])

However, the test vector in this case is the reverse of the definition quoted above (since y is +ve through lon=90). If this is correct, there are a couple of options:

• Leave conversion as it is, and update comments / test to correctly reference directions (that the resultant geog reference frame is North,West,Out).
• Change the conversion to match the documentation. I don't know what other codes this would impact. The resulting reference frame is arguably more sensible, though it is not RH rule compliant (which N,W,Out is).

Thoughts?

[andreww]

I've just seen this on slack and come to the same conclusion - we seem to have a geographical convention that is U_lat (positive towards the north), U_lon (positive towards the west) and U_r (positive outwards), and after much staring at the fingers on my right hand and twisting my wrist in odd directions I decided that this is a right handed system.

I kind of like things being on a right handed system, so I would go with either (1) keeping the current 'geographical' reference frame or (2) flipping the E-W and up-down to give the right handed frame of U_lat (positive towards the north), U_lon (positive towards the east) and U_r (positive inwards).

For (1) we should also fix the documentation (a figure would be nice) and swap the names of those functions in the test. (2) also needs a change in the rotation matrix.

It's probably worth also changing the name of the field from "u_geog" to "u_nwu" or "u_ned". It would be relatively easy to interpret combinations of letters n, e, s, w, u and d in a sensible way to build the rotation matrix, and let the user decide... but that's probably more effort than it's worth.

[anowacki]

after much staring at the fingers on my right hand and twisting my wrist in odd directions I decided that this is a right handed system.

~Actually, that is a left-handed system!~ Sorry, it is a left-handed system if you have positive-west. But really who does that? Can we instead settle on (east, north, up)?

[anowacki]

I am also happy with u_enu, rather than the existing u_geog.

[anowacki]

As a final thought, our current public API is consistent with using the order (lon, lat[, r]) for geographic things, and since more people are currently using that than the internal stuff, I think that's another reason to move to having a local flow system of (lon, lat, r) = (east, north, up) everywhere.

[jwookey]

I agree that E-N-U makes the most sense, and I had noticed the inconsistency with other parts of the code. I'm happy to implement these changes, and make a pull request.

[anowacki]

I'm happy to implement these changes, and make a pull request.

Thanks!

[eejwa]

I agree that E-N-U makes the most sense, and I had noticed the inconsistency with other parts of the code. I'm happy to implement these changes, and make a pull request.

Thank you for cleaning up after me. I'm happy to help however I can or stay out of the way for this one...

[anowacki]

Is this addressed by #108 do you think, @jwookey?

[anowacki]

Actually, yes it is.