-
Notifications
You must be signed in to change notification settings - Fork 0
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
Bugfixes for transform_itrf
#45
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -47,8 +47,10 @@ def transform_itrf( | |
# and the mean of that chunk is used to determine the plate name. | ||
plate: str | None = None, | ||
) -> IceflowDataFrame: | ||
"""Pipeline string for proj to transform from the source to the target | ||
ITRF frame and, optionally, epoch. | ||
"""Transform the data's lon/lat/elev from the source ITRF to the target ITRF. | ||
|
||
If a `target_epoch` is given, coordinate propagation is performed via a | ||
plate motion model. | ||
""" | ||
if not check_itrf(target_itrf): | ||
err_msg = ( | ||
|
@@ -60,7 +62,7 @@ def transform_itrf( | |
transformed_chunks = [] | ||
for source_itrf, chunk in data.groupby(by="ITRF"): | ||
# If the source ITRF is the same as the target for this chunk, skip transformation. | ||
if source_itrf == target_itrf: | ||
if source_itrf == target_itrf and target_epoch is None: | ||
transformed_chunks.append(chunk) | ||
continue | ||
|
||
|
@@ -69,17 +71,51 @@ def transform_itrf( | |
if not plate: | ||
plate = plate_name(Point(chunk.longitude.mean(), chunk.latitude.mean())) | ||
plate_model_step = ( | ||
f"+step +inv +init={target_itrf}:{plate} +t_epoch={target_epoch} " | ||
# Perform coordinate propagation to the given epoch using the | ||
# provided plate motion model (PMM). An example is given in the | ||
# message of this commit: | ||
# https://github.com/OSGeo/PROJ/commit/403f930355926aced5caba5bfbcc230ad152cf86 | ||
f"+step +init={target_itrf}:{plate} +t_epoch={target_epoch} " | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've removed the The See the linked commit's message for an example, which does not utilize the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a complete guess, but maybe we put the
We do this for the ITRF transformation step because the So our pipeline is often constructed to look like this:
This looks up the ITRF2014 data file and finds the TIRF2008 transformation parameters (parameters to go from ITRF2014 to ITRF2008). By adding the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here's a proj mailing list thread about the correct syntax for plate motion models: https://lists.osgeo.org/pipermail/proj/2022-February/010527.html In the example, a set of coordinates at epoch 2014.0 are propagated to 2022.0 using the ITRF2014:EURA model:
This would seem to confirm that the But later, in this message from Even: https://lists.osgeo.org/pipermail/proj/2022-February/010530.html
|
||
) | ||
|
||
itrf_transformation_step = "" | ||
if source_itrf != target_itrf: | ||
# This performs a helmert transform (see | ||
# https://proj.org/en/9.4/operations/transformations/helmert.html). `+init=ITRF2014:ITRF2008` | ||
# looks up the ITRF2008 helmert transformation step in the ITRF2014 | ||
# data file (see | ||
# https://proj.org/en/9.3/resource_files.html#init-files and e.g., | ||
# https://github.com/OSGeo/PROJ/blob/master/data/ITRF2014). The | ||
# `+inv` reverses the transformation. So `+init=ITRF2014:ITRF2008` | ||
# performs a helmert transform from ITRF2008 to ITRF2014. | ||
itrf_transformation_step = f"+step +inv +init={target_itrf}:{source_itrf} " | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I want to think more about how we can support ITRF transformations more broadly. E.g., if someone passes in a I think the reason we did it this way in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could have some code that tests each way for validity. E.g., if This may not always work however. To make things a little more complicated, the data files that make the The latest version of A user could construct their own pipeline and pass in the helmert transformation parameters directly, or maybe we add that as an option here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. WIP implementation that tests each way (with and without |
||
|
||
pipeline = ( | ||
# This initializes the pipeline and declares the use of the WGS84 | ||
# ellipsoid for all of the following steps. See | ||
# https://proj.org/en/9.5/operations/pipeline.html. | ||
f"+proj=pipeline +ellps=WGS84 " | ||
# Performs unit conversion from lon/lat degrees to radians. | ||
# TODO: This step appears to be unnecessary. Removing it does not appear to | ||
# affect the output. The following steps require that the | ||
# coordinates be geodedic, which could be radians or degrees. | ||
f"+step +proj=unitconvert +xy_in=deg +xy_out=rad " | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did some testing and this step does not appear to be necessary. Proj seems to assume lat/lon inputs by default and is "unit-aware" in that it will raise an error between steps if it detects an incompatibility. I don't think it hurts to be here, but if not necessary I'm inclined to remove it to simplify the transformation.
is the same as:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we got the outline from our pipeline from this response from Even Rouault in a proj thread that I started: https://lists.osgeo.org/pipermail/proj/2019-May/008509.html In a subsequent message Alan Snow notes:
Which would seem to suggest that the unit conversion step could be skipped. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The examples given in this PR to add the ITRF2020 params to proj shows the unit conversion steps. This may be a best-practice. See OSGeo/PROJ#4235. |
||
# This step explicitly sets the projection as lat/lon. It won't | ||
# change the coordinates, but they will be identified as geodetic, | ||
# which is necessary for the next steps. | ||
f"+step +proj=latlon " | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't do anything aside from declare the coordinates to be geodetic, which is required by the subsequent steps. Not technically necessary but maybe better to be explicit in this case? |
||
# Convert from lat/lon/elev geodetic coordinates to cartesian | ||
# coordinates, which are required for the following steps. | ||
# See: https://proj.org/en/9.5/operations/conversions/cart.html | ||
f"+step +proj=cart " | ||
f"+step +inv +init={target_itrf}:{source_itrf} " | ||
# ITRF transformation. See above for definition. | ||
f"{itrf_transformation_step}" | ||
# See above for definition. | ||
f"{plate_model_step}" | ||
# Convert back from cartesian to lat/lon coordinates | ||
f"+step +inv +proj=cart " | ||
# Convert lon/lat from radians back to degrees. | ||
# TODO: remove this if the initial conversion to radians above is not needed | ||
f"+step +proj=unitconvert +xy_in=rad +xy_out=deg" | ||
) | ||
|
||
|
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.
We only skip transforming a chunk if both the source and target ITRF match, and no target epoch is given.
This allows us to do plate-motion coordinate propagation within a single ITRF (e.g., IS2 is in ITRF2014, and we want to compare to other data in ITRF2014, but we want to be able to change the epoch to account for plate motion over time).