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

Use hvPlot in Plotting API, Add Polygon Projection Support #922

Merged
merged 74 commits into from
Oct 2, 2024

Conversation

philipc2
Copy link
Member

@philipc2 philipc2 commented Aug 30, 2024

Closes #849 #493 #918 #625

Overview

  • Updates plotting API to use hvPlot
  • Adds support for Cartopy Projections when plotting polygons (raster & vector)
  • Updated to_geoataframe() methods to better align with recent changes to to_polycollection()
  • Adds support for directly constructing a geopandas.GeoDataFrame using the engine='geopandas' parameter

@philipc2 philipc2 added the run-benchmark Run ASV benchmark workflow label Aug 30, 2024
Copy link

github-actions bot commented Aug 30, 2024

ASV Benchmarking

Benchmark Comparison Results

Benchmarks that have improved:

Change Before [8b1c3de] After [37abcf2] Ratio Benchmark (Parameter)
- 1.41±0.02μs 1.21±0.01μs 0.86 mpas_ocean.ConstructTreeStructures.time_ball_tree('120km')
- 969±20ns 870±10ns 0.9 mpas_ocean.ConstructTreeStructures.time_kd_tree('120km')

Benchmarks that have stayed the same:

Change Before [8b1c3de] After [37abcf2] Ratio Benchmark (Parameter)
1.59±0s 1.61±0.01s 1.01 face_bounds.FaceBounds.time_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/mpas/QU/oQU480.231010.nc'))
225±0.2ms 224±0.09ms 1.00 face_bounds.FaceBounds.time_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/scrip/outCSne8/outCSne8.nc'))
2.03±0s 2.07±0s 1.02 face_bounds.FaceBounds.time_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/ugrid/geoflow-small/grid.nc'))
7.57±0.07ms 7.65±0.04ms 1.01 face_bounds.FaceBounds.time_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/ugrid/quad-hexagon/grid.nc'))
665±5μs 668±9μs 1.01 mpas_ocean.CheckNorm.time_check_norm('120km')
433±6μs 448±8μs 1.03 mpas_ocean.CheckNorm.time_check_norm('480km')
618±6ms 631±1ms 1.02 mpas_ocean.ConnectivityConstruction.time_face_face_connectivity('120km')
40.9±0.3ms 41.6±0.9ms 1.02 mpas_ocean.ConnectivityConstruction.time_face_face_connectivity('480km')
2.04±0.07ms 1.82±0.2ms ~0.89 mpas_ocean.ConnectivityConstruction.time_n_nodes_per_face('120km')
495±10μs 498±20μs 1.01 mpas_ocean.ConnectivityConstruction.time_n_nodes_per_face('480km')
342±30ns 315±1ns 0.92 mpas_ocean.ConstructTreeStructures.time_ball_tree('480km')
321±20ns 299±10ns 0.93 mpas_ocean.ConstructTreeStructures.time_kd_tree('480km')
1.06±0.01s 1.04±0.01s 0.98 mpas_ocean.GeoDataFrame.time_to_geodataframe('120km', False)
57.9±0.3ms 54.8±0.7ms 0.95 mpas_ocean.GeoDataFrame.time_to_geodataframe('120km', True)
78.5±2ms 77.5±0.6ms 0.99 mpas_ocean.GeoDataFrame.time_to_geodataframe('480km', False)
5.28±0.1ms 5.08±0.08ms 0.96 mpas_ocean.GeoDataFrame.time_to_geodataframe('480km', True)
2.69±0.01ms 2.69±0ms 1.00 mpas_ocean.Gradient.time_gradient('120km')
288±0.6μs 289±2μs 1.00 mpas_ocean.Gradient.time_gradient('480km')
248±4μs 245±8μs 0.99 mpas_ocean.HoleEdgeIndices.time_construct_hole_edge_indices('120km')
123±1μs 124±1μs 1.01 mpas_ocean.HoleEdgeIndices.time_construct_hole_edge_indices('480km')
407M 407M 1.00 mpas_ocean.Integrate.peakmem_integrate('480km')
182±10ms 176±0.5ms 0.97 mpas_ocean.Integrate.time_integrate('120km')
11.7±0.06ms 12.1±0.4ms 1.03 mpas_ocean.Integrate.time_integrate('480km')
346±2ms 350±2ms 1.01 mpas_ocean.MatplotlibConversion.time_dataarray_to_polycollection('120km', 'exclude')
346±2ms 346±2ms 1.00 mpas_ocean.MatplotlibConversion.time_dataarray_to_polycollection('120km', 'include')
356±4ms 346±3ms 0.97 mpas_ocean.MatplotlibConversion.time_dataarray_to_polycollection('120km', 'split')
21.2±0.1ms 23.0±0.3ms 1.08 mpas_ocean.MatplotlibConversion.time_dataarray_to_polycollection('480km', 'exclude')
21.6±0.2ms 22.6±0.4ms 1.04 mpas_ocean.MatplotlibConversion.time_dataarray_to_polycollection('480km', 'include')
21.6±0.1ms 22.6±0.4ms 1.05 mpas_ocean.MatplotlibConversion.time_dataarray_to_polycollection('480km', 'split')
55.2±0.07ms 55.4±0.1ms 1.00 mpas_ocean.RemapDownsample.time_inverse_distance_weighted_remapping
44.7±0.3ms 44.5±0.02ms 1.00 mpas_ocean.RemapDownsample.time_nearest_neighbor_remapping
363±2ms 363±1ms 1.00 mpas_ocean.RemapUpsample.time_inverse_distance_weighted_remapping
266±0.6ms 265±0.3ms 0.99 mpas_ocean.RemapUpsample.time_nearest_neighbor_remapping
6.59±0.07ms 6.61±0.02ms 1.00 quad_hexagon.QuadHexagon.time_open_dataset
5.66±0.02ms 5.69±0.06ms 1.01 quad_hexagon.QuadHexagon.time_open_grid

Benchmarks that have got worse:

Change Before [8b1c3de] After [37abcf2] Ratio Benchmark (Parameter)
+ 380M 440M 1.16 face_bounds.FaceBounds.peakmem_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/mpas/QU/oQU480.231010.nc'))
+ 381M 440M 1.15 face_bounds.FaceBounds.peakmem_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/scrip/outCSne8/outCSne8.nc'))
+ 384M 442M 1.15 face_bounds.FaceBounds.peakmem_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/ugrid/geoflow-small/grid.nc'))
+ 383M 440M 1.15 face_bounds.FaceBounds.peakmem_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/ugrid/quad-hexagon/grid.nc'))
+ 1.69±0.01s 2.95±0.02s 1.74 import.Imports.timeraw_import_uxarray
+ 263M 316M 1.2 mpas_ocean.Gradient.peakmem_gradient('120km')
+ 240M 294M 1.22 mpas_ocean.Gradient.peakmem_gradient('480km')
+ 369M 423M 1.15 mpas_ocean.Integrate.peakmem_integrate('120km')
+ 236M 290M 1.23 quad_hexagon.QuadHexagon.peakmem_open_dataset
+ 235M 289M 1.23 quad_hexagon.QuadHexagon.peakmem_open_grid

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@philipc2 philipc2 marked this pull request as ready for review September 4, 2024 14:31
@philipc2 philipc2 changed the title DRAFT: Add Projection Support for Polygons Add Projection Support for Polygons Sep 4, 2024
@philipc2 philipc2 self-assigned this Sep 4, 2024
@aaronzedwick
Copy link
Member

Is there a better mesh you could use for the notebook example? Maybe with more cells? And possibly without the holes? It can be confusing to look at. Also now that I am thinking about it, we should start using the UXarray colormaps for the notebooks.

@philipc2
Copy link
Member Author

philipc2 commented Sep 5, 2024

Is there a better mesh you could use for the notebook example? Maybe with more cells? And possibly without the holes? It can be confusing to look at. Also now that I am thinking about it, we should start using the UXarray colormaps for the notebooks.

Agree with all the points here. I thought the PR was ready for review (but quickly turned it back to draft). I'll address this points!

uxarray/grid/grid.py Outdated Show resolved Hide resolved
uxarray/grid/grid.py Outdated Show resolved Hide resolved
Co-authored-by: Orhan Eroglu <[email protected]>
Copy link
Member

@erogluorhan erogluorhan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've resolved the previous comments and now added a few more as part of my second round of review

@philipc2
Copy link
Member Author

philipc2 commented Oct 1, 2024

I've resolved the previous comments and now added a few more as part of my second round of review

Thank you!

Copy link
Member

@erogluorhan erogluorhan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am now good with all of the code here.

This PR seems to be reducing the code coverage though, and some of them looks real (e.g. _correct_central_longitude or part of it does not seem to be covered). I might be missing some contextvfrom the rest of the review process though. Could you clarify?

@philipc2
Copy link
Member Author

philipc2 commented Oct 2, 2024

I am now good with all of the code here.

This PR seems to be reducing the code coverage though, and some of them looks real (e.g. _correct_central_longitude or part of it does not seem to be covered). I might be missing some contextvfrom the rest of the review process though. Could you clarify?

Thanks for the comprehensive review! I've made a few changes since writing the initial tests as I've been addressing the reviews. I'll get those added and look into improving the code coverage.

@philipc2 philipc2 added this to the Visualization milestone Oct 2, 2024
@philipc2 philipc2 added the run-benchmark Run ASV benchmark workflow label Oct 2, 2024
Copy link
Member

@erogluorhan erogluorhan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great work and marks a visualization milestone with the projections support it brings!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-benchmark Run ASV benchmark workflow
Projects
None yet
4 participants