Skip to content
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

sliderule: update to EPSG:7912, use 3D Point geometry #272

Merged
merged 5 commits into from
Sep 6, 2023

Conversation

dshean
Copy link
Member

@dshean dshean commented Jun 29, 2023

Addresses some of the issues in #198

After some experimentation with latest PROJ (9.2.1) and GeoPandas, it appears that if we include 3D Point Geometries, we can do the 3D CRS transformations using the to_crs() functionality, since we now assign EPSG:7912 (3D CRS for ITRF2014 realization).

This PR updates the default CRS for the Python client output (was inconsistent with what is returned by the server-side GeoRaster code). Also updated a few comments (EPSG:4326 is not MERCATOR), and fixed a typo.

There may be some unforeseen gotchas with the 3D Points, and we are preserving a redundant copy of the h_mean attribute as the Z coordinate in the Point, but I think this is acceptable. We don't want to drop the h_mean column. An alternative would be to allow the user to specify whether they want a 2D or 3D Point geometry column for the GeoDataFrame.

@dshean
Copy link
Member Author

dshean commented Jun 29, 2023

I may have missed other instances where the CRS is specified. I see the following:
https://github.com/search?q=repo%3AICESat2-SlideRule%2Fsliderule+SLIDERULE_EPSG&type=code

@dshean
Copy link
Member Author

dshean commented Jun 30, 2023

OK, tests failed. I assumed h_mean would be in the columns

@jpswinski
Copy link
Member

@dshean I made the necessary updates to the Python client so that the elevation column of each of the datasets could be used.

But, I am seeing that the runs are taking much longer. A 30 second ATL06-SR request, with the 3D transformation, takes a 150 seconds. I think there may be a lot of use cases (like the web-client), where the added time may not be needed and will artificially slow things down. What do you think?

My recommendation is to have this be a user-enabled feature. When the user specifies the height_key in their request, it will trigger the client using it to perform the 3D transformation. That is the behavior as of the last commit.

@scottyhq
Copy link
Contributor

If there is a significant performance hit that might merit keeping the 2D geometry by default. Here is a link to some demos doing client-side promotion to 3D, which works well for small datasets, but probably not for large geoparquet outputs (https://github.com/ICESAT-2HackWeek/3D_CRS_Transformation_Resources/blob/main/geopandas3D.ipynb). for the api, matching geopandas and shapely defaults where possible makes sense to me. For example, maybe sliderule requests set include_z=False by default but setting to True triggers 3D geometries returned from the server (https://shapely.readthedocs.io/en/stable/reference/shapely.get_coordinates.html)

@jpswinski jpswinski merged commit 468c2f7 into main Sep 6, 2023
4 of 5 checks passed
@jpswinski jpswinski deleted the gpd_3D_geometry branch September 6, 2023 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants