-
Notifications
You must be signed in to change notification settings - Fork 20
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
[mock_uss, monitorlib] Add EGM96 datum conversion #342
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me 👍 Could you please add the scale on the plot you attached to the PR to help readers evaluate the accuracy?
monitoring/monitorlib/geo.py
Outdated
grid_path = os.path.join(os.path.dirname(__file__), "assets/WW15MGH.DAC") | ||
grid = ( | ||
np.fromfile(grid_path, ">i2").reshape( | ||
int(180 / grid_size + 1), int(360 / grid_size) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not very clear why you added a buffer to the latitude argument and not for longitude, could you please clarify and perhaps add it in comments?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've switched to use enumerated latitudes and longitudes rather than counting them separately here.
monitoring/monitorlib/geo.py
Outdated
/ 100 | ||
) | ||
_egm96 = Spline( | ||
np.arange(-90, 90 + grid_size / 2, grid_size), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the previous response will clarify the latitude buffer here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The latitude enumeration now has a comment that it's on the [-90, 90] interval. arange
does not have an option for an inclusive endpoint, so the targeted endpoint must extend further than the desired endpoint, but not so far as to reach the next step. Adding one half-step works for this.
lat = p.lat().degrees | ||
if lat < -90 or lat > 90: | ||
raise ValueError(f"Cannot compute EGM96 geoid offset at latitude {lat} degrees") | ||
return _egm96.ev(-lat, lng) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please clarify in a comment why latitude needs to be inverted here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added
Done; comment updated. |
* Add EGM96 datum conversion * make format * Address comments and `make format` 2825146
* Add EGM96 datum conversion * make format * Address comments and `make format` 2825146
U-space requires USSPs to deliver messages with MSL altitude (using EGM96 or EGM2008 geoid as the datum) to authorised users while ASTM F3411-22a transmits altitude between USSPs using the WGS84 ellipsoid as the datum. In preparation to incorporate MSL altitude observations, this PR implements an algorithm to determine the EGM96 geoid offset from the WGS84 ellipsoid and uses it to compute MSL altitude in mock_uss.
The computation is performed as part of the CI (during the NetRID tests). To verify accuracy, I used the following code:
The absolute accuracy of one point was manually verified (34,-118 -> -34.16 meters, versus this calculator), and then the overall shape was verified visually using this output: