Skip to content

fix time_face_search benchmark#1546

Merged
rajeeja merged 2 commits into
mainfrom
fix-benchmark-time-face-search
Jul 6, 2026
Merged

fix time_face_search benchmark#1546
rajeeja merged 2 commits into
mainfrom
fix-benchmark-time-face-search

Conversation

@Sevans711

@Sevans711 Sevans711 commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

Closes #1545

Overview

Fixes benchmarks:

mpas_ocean.PointInPolygon.time_face_search('120km')
mpas_ocean.PointInPolygon.time_face_search('480km')

which were failing with: TypeError: Grid.get_faces_containing_point() got an unexpected keyword argument 'point_xyz'

PR Checklist

General

  • An issue is linked created and linked
  • Add appropriate labels
  • Filled out Overview and Expected Usage (if applicable) sections

@Sevans711 Sevans711 added bug Something isn't working benchmarking Related to benchmarks, memory usage, and/or time profiling run-benchmark Run ASV benchmark workflow labels Jul 6, 2026
@github-actions

github-actions Bot commented Jul 6, 2026 •
edited
Loading

Copy link
Copy Markdown

ASV Benchmarking

Benchmark Comparison Results

Benchmarks that have improved:

Change Before [df6ef32] After [d309620] Ratio Benchmark (Parameter)
- 631M 441M 0.7 face_bounds.FaceBounds.peakmem_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/ugrid/geoflow-small/grid.nc'))
- 753M 440M 0.58 face_bounds.FaceBounds.peakmem_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/ugrid/quad-hexagon/grid.nc'))
- 551M 433M 0.79 mpas_ocean.Gradient.peakmem_gradient('480km')

Benchmarks that have stayed the same:

Change Before [df6ef32] After [d309620] Ratio Benchmark (Parameter)
439M 439M 1 face_bounds.FaceBounds.peakmem_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/mpas/QU/oQU480.231010.nc'))
469M 469M 1 face_bounds.FaceBounds.peakmem_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/scrip/outCSne8/outCSne8.nc'))
12.8±0.03ms 12.9±0.04ms 1 face_bounds.FaceBounds.time_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/mpas/QU/oQU480.231010.nc'))
3.44±0.08ms 3.37±0.08ms 0.98 face_bounds.FaceBounds.time_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/scrip/outCSne8/outCSne8.nc'))
17.3±0.08ms 17.3±0.08ms 1 face_bounds.FaceBounds.time_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/ugrid/geoflow-small/grid.nc'))
2.01±0.04ms 1.97±0.03ms 0.98 face_bounds.FaceBounds.time_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/ugrid/quad-hexagon/grid.nc'))
4.56±0.02s 4.52±0s 0.99 import.Imports.timeraw_import_uxarray
892±30ns 905±20ns 1.01 mpas_ocean.CheckNorm.time_check_norm('120km')
852±6ns 847±4ns 1 mpas_ocean.CheckNorm.time_check_norm('480km')
719±9ms 714±2ms 0.99 mpas_ocean.ConnectivityConstruction.time_face_face_connectivity('120km')
45.3±0.1ms 44.8±0.3ms 0.99 mpas_ocean.ConnectivityConstruction.time_face_face_connectivity('480km')
621±9μs 620±10μs 1 mpas_ocean.ConnectivityConstruction.time_n_nodes_per_face('120km')
551±10μs 553±10μs 1 mpas_ocean.ConnectivityConstruction.time_n_nodes_per_face('480km')
5.07±0.01ms 5.02±0.05ms 0.99 mpas_ocean.ConstructFaceLatLon.time_cartesian_averaging('120km')
3.52±0.01ms 3.52±0.03ms 1 mpas_ocean.ConstructFaceLatLon.time_cartesian_averaging('480km')
3.31±0.01s 3.29±0.04s 1 mpas_ocean.ConstructFaceLatLon.time_welzl('120km')
212±1ms 212±0.6ms 1 mpas_ocean.ConstructFaceLatLon.time_welzl('480km')
19.7±0.02ms 19.7±0.03ms 1 mpas_ocean.ConstructTreeStructures.time_ball_tree('120km')
1.05±0.02ms 1.06±0.01ms 1.01 mpas_ocean.ConstructTreeStructures.time_ball_tree('480km')
10.6±0.02ms 10.6±0.01ms 1 mpas_ocean.ConstructTreeStructures.time_kd_tree('120km')
676±8μs 683±10μs 1.01 mpas_ocean.ConstructTreeStructures.time_kd_tree('480km')
655±1ms 648±4ms 0.99 mpas_ocean.CrossSections.time_const_lat('120km', 1)
329±1ms 330±0.5ms 1 mpas_ocean.CrossSections.time_const_lat('120km', 2)
171±1ms 171±0.3ms 1 mpas_ocean.CrossSections.time_const_lat('120km', 4)
487±0.6ms 489±1ms 1 mpas_ocean.CrossSections.time_const_lat('480km', 1)
248±0.3ms 248±0.4ms 1 mpas_ocean.CrossSections.time_const_lat('480km', 2)
126±0.2ms 126±0.3ms 1 mpas_ocean.CrossSections.time_const_lat('480km', 4)
22.0±0.04ms 22.0±0.05ms 1 mpas_ocean.DualMesh.time_dual_mesh_construction('120km')
2.55±0.01ms 2.54±0.02ms 0.99 mpas_ocean.DualMesh.time_dual_mesh_construction('480km')
848±6ms 836±4ms 0.99 mpas_ocean.GeoDataFrame.time_to_geodataframe('120km', False)
50.1±0.8ms 50.7±0.7ms 1.01 mpas_ocean.GeoDataFrame.time_to_geodataframe('120km', True)
72.6±0.3ms 72.6±0.7ms 1 mpas_ocean.GeoDataFrame.time_to_geodataframe('480km', False)
5.31±0.08ms 5.27±0.04ms 0.99 mpas_ocean.GeoDataFrame.time_to_geodataframe('480km', True)
453M 454M 1 mpas_ocean.Gradient.peakmem_gradient('120km')
165±0.3ms 165±4ms 1 mpas_ocean.Gradient.time_gradient('120km')
11.5±0.07ms 11.4±0ms 0.99 mpas_ocean.Gradient.time_gradient('480km')
188±2μs 190±2μs 1.01 mpas_ocean.HoleEdgeIndices.time_construct_hole_edge_indices('120km')
88.0±0.7μs 88.6±2μs 1.01 mpas_ocean.HoleEdgeIndices.time_construct_hole_edge_indices('480km')
403M 403M 1 mpas_ocean.Integrate.peakmem_integrate('120km')
382M 382M 1 mpas_ocean.Integrate.peakmem_integrate('480km')
178±0.5μs 179±1μs 1 mpas_ocean.Integrate.time_integrate('120km')
158±1μs 156±0.3μs 0.99 mpas_ocean.Integrate.time_integrate('480km')
132±1ms 133±2ms 1.01 mpas_ocean.MatplotlibConversion.time_dataarray_to_polycollection('120km', 'exclude')
132±0.6ms 132±0.9ms 1 mpas_ocean.MatplotlibConversion.time_dataarray_to_polycollection('120km', 'include')
132±1ms 132±1ms 1 mpas_ocean.MatplotlibConversion.time_dataarray_to_polycollection('120km', 'split')
10.3±0.06ms 10.4±0.04ms 1 mpas_ocean.MatplotlibConversion.time_dataarray_to_polycollection('480km', 'exclude')
10.3±0.03ms 10.3±0.03ms 1 mpas_ocean.MatplotlibConversion.time_dataarray_to_polycollection('480km', 'include')
10.9±0.7ms 10.4±0.08ms 0.95 mpas_ocean.MatplotlibConversion.time_dataarray_to_polycollection('480km', 'split')
287±2μs 284±2μs 0.99 mpas_ocean.PointInPolygon.time_face_search_lonlat('120km')
285±1μs 285±1μs 1 mpas_ocean.PointInPolygon.time_face_search_lonlat('480km')
267±1μs 266±0.9μs 1 mpas_ocean.PointInPolygon.time_face_search_xyz('120km')
270±2μs 270±2μs 1 mpas_ocean.PointInPolygon.time_face_search_xyz('480km')
211±1ms 211±1ms 1 mpas_ocean.RemapDownsample.time_bilinear_remapping
230±3ms 229±1ms 1 mpas_ocean.RemapDownsample.time_inverse_distance_weighted_remapping
4.25±0.01ms 4.26±0.02ms 1 mpas_ocean.RemapDownsample.time_nearest_neighbor_remapping
1.19±0.01s 1.19±0s 1 mpas_ocean.RemapUpsample.time_bilinear_remapping
35.3±0.4ms 35.4±0.3ms 1 mpas_ocean.RemapUpsample.time_inverse_distance_weighted_remapping
8.93±0.1ms 9.02±0.1ms 1.01 mpas_ocean.RemapUpsample.time_nearest_neighbor_remapping
29.0±0.4ms 27.8±0.08ms 0.96 mpas_ocean.ZonalAverage.time_zonal_average('120km')
6.10±0.07ms 5.90±0.03ms 0.97 mpas_ocean.ZonalAverage.time_zonal_average('480km')
380M 378M 0.99 quad_hexagon.QuadHexagon.peakmem_open_dataset
377M 377M 1 quad_hexagon.QuadHexagon.peakmem_open_grid
6.42±0.1ms 6.47±0.08ms 1.01 quad_hexagon.QuadHexagon.time_open_dataset
5.41±0.05ms 5.45±0.05ms 1.01 quad_hexagon.QuadHexagon.time_open_grid

@Sevans711 Sevans711 mentioned this pull request Jul 6, 2026
Open
@Sevans711 Sevans711 requested review from cmdupuis3 and rajeeja July 6, 2026 17:45
rajeeja
rajeeja requested changes Jul 6, 2026
View reviewed changes

@rajeeja rajeeja left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for looking into this. Can you try moving point_xyz and point_lonlat construction into setup() so the timed methods only measure get_faces_containing_point()?

@Sevans711
improve time_face_search benchmark setup
422de26 
moves more construction into setup() to ensure the timed methods only measure get_faces_containing_point()
@Sevans711

Sevans711 commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator Author

@rajeeja thank you for reviewing! I think 422de26 should work. Could you take another look when you get the chance?

rajeeja
rajeeja approved these changes Jul 6, 2026
View reviewed changes

@rajeeja rajeeja left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, this looks good now.

@rajeeja rajeeja merged commit ad735a8 into main Jul 6, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rajeeja rajeeja rajeeja approved these changes
@cmdupuis3 cmdupuis3 Awaiting requested review from cmdupuis3

Assignees

No one assigned

Labels

benchmarking Related to benchmarks, memory usage, and/or time profiling bug Something isn't working run-benchmark Run ASV benchmark workflow

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Benchmark mpas_ocean.PointInPolygon.time_face_search is failing

2 participants

@Sevans711 @rajeeja