Skip to content

Connectivity benchmarks: gate larger files - #1550

Merged
cmdupuis3 merged 4 commits into
UXARRAY:mainfrom
cmdupuis3:cmdupuis3/connectivity_benchmarks
Jul 9, 2026
Merged

Connectivity benchmarks: gate larger files#1550
cmdupuis3 merged 4 commits into
UXARRAY:mainfrom
cmdupuis3:cmdupuis3/connectivity_benchmarks

Conversation

@cmdupuis3

@cmdupuis3cmdupuis3 commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

This PR is to fix an issue with the connectivity benchmarks. The current github runner doesn't seem to have access to the larger dyamond files, so those benchmarks fail and spam up the benchmark logs.

Continuation of #1549.

Overview

I naively copied the file paths from the dyamond benchmarks without the gate, not realizing that the github runner wouldn't have access to them. The gate there basically pings for file access, which will fail for github's runner, but would succeed for someone running somewhere with transparent access to the data on Glade.

PR Checklist

General

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

Testing

  • [N/A] Adequate tests are created if there is new functionality
  • [N/A] Tests cover all possible logical paths in your function
  • [N/A] Tests are not too basic (such as simply calling a function and nothing else)

Documentation

  • Docstrings have been added to all new functions
  • Docstrings have updated with any function changes
  • Internal functions have a preceding underscore (_) and have been added to docs/internal_api/index.rst
  • User functions have been added to docs/user_api/index.rst

Examples

  • Any new notebook examples added to docs/examples/ folder
  • Clear the output of all cells before committing
  • New notebook files added to docs/examples.rst toctree
  • New notebook files added to new entry in docs/gallery.yml with appropriate thumbnail photo in docs/_static/thumbnails/

@cmdupuis3cmdupuis3 added benchmarking Related to benchmarks, memory usage, and/or time profiling run-benchmark Run ASV benchmark workflow labels Jul 7, 2026
@cmdupuis3
cmdupuis3 requested a review from rajeejaJuly 7, 2026 18:22
@cmdupuis3
cmdupuis3 marked this pull request as draft July 7, 2026 19:21
@cmdupuis3
cmdupuis3 marked this pull request as ready for review July 7, 2026 20:24
@Sevans711

Copy link
Copy Markdown
Collaborator

Note, looks like benchmarks seem to be "failing" here for the reason discussed in issue #1547 (also affecting PR #1541).

@github-actions

Copy link
Copy Markdown

ASV Benchmarking

Benchmark Comparison Results

Benchmarks that have improved:

ChangeBefore [7c9b487]After [b85be3a]RatioBenchmark (Parameter)
-630M441M0.7face_bounds.FaceBounds.peakmem_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/ugrid/geoflow-small/grid.nc'))
-751M440M0.59face_bounds.FaceBounds.peakmem_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/ugrid/quad-hexagon/grid.nc'))
-550M433M0.79mpas_ocean.Gradient.peakmem_gradient('480km')

Benchmarks that have stayed the same:

ChangeBefore [7c9b487]After [b85be3a]RatioBenchmark (Parameter)
10.8±0.07μs10.7±0.3μs0.99bench_connectivity.Connectivity.time_edge_face('120km')
11.0±0.2μs11.1±0.1μs1.01bench_connectivity.Connectivity.time_edge_face('480km')
10.7±0.2μs10.8±0.06μs1.01bench_connectivity.Connectivity.time_edge_node('120km')
11.0±0.09μs11.0±0.1μs1.00bench_connectivity.Connectivity.time_edge_node('480km')
10.7±0.05μs11.0±0.1μs1.03bench_connectivity.Connectivity.time_face_edge('120km')
11.0±0.1μs11.0±0.04μs1.00bench_connectivity.Connectivity.time_face_edge('480km')
10.8±0.1μs11.0±0.1μs1.02bench_connectivity.Connectivity.time_face_face('120km')
11.2±0.1μs10.9±0.1μs0.97bench_connectivity.Connectivity.time_face_face('480km')
22.0±0.7μs21.8±0.3μs0.99bench_connectivity.Connectivity.time_face_node('120km')
22.2±0.3μs22.0±0.1μs0.99bench_connectivity.Connectivity.time_face_node('480km')
11.0±0.07μs10.6±0.2μs0.96bench_connectivity.Connectivity.time_node_edge('120km')
11.0±0.1μs10.9±0.2μs0.99bench_connectivity.Connectivity.time_node_edge('480km')
10.8±0.3μs10.7±0.07μs0.99bench_connectivity.Connectivity.time_node_face('120km')
11.0±0.2μs11.2±0.08μs1.02bench_connectivity.Connectivity.time_node_face('480km')
439M439M1.00face_bounds.FaceBounds.peakmem_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/mpas/QU/oQU480.231010.nc'))
469M469M1.00face_bounds.FaceBounds.peakmem_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/scrip/outCSne8/outCSne8.nc'))
14.8±0.07ms15.1±0.1ms1.02face_bounds.FaceBounds.time_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/mpas/QU/oQU480.231010.nc'))
3.76±0.04ms3.76±0.05ms1.00face_bounds.FaceBounds.time_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/scrip/outCSne8/outCSne8.nc'))
19.1±0.3ms19.0±0.04ms1.00face_bounds.FaceBounds.time_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/ugrid/geoflow-small/grid.nc'))
2.21±0.02ms2.21±0.05ms1.00face_bounds.FaceBounds.time_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/ugrid/quad-hexagon/grid.nc'))
4.85±0.01s4.98±0.02s1.03import.Imports.timeraw_import_uxarray
943±9ns960±5ns1.02mpas_ocean.CheckNorm.time_check_norm('120km')
907±5ns893±4ns0.98mpas_ocean.CheckNorm.time_check_norm('480km')
839±3ms872±20ms1.04mpas_ocean.ConnectivityConstruction.time_face_face_connectivity('120km')
53.7±0.8ms55.1±0.5ms1.03mpas_ocean.ConnectivityConstruction.time_face_face_connectivity('480km')
703±10μs679±7μs0.97mpas_ocean.ConnectivityConstruction.time_n_nodes_per_face('120km')
605±10μs641±10μs1.06mpas_ocean.ConnectivityConstruction.time_n_nodes_per_face('480km')
5.43±0.02ms5.46±0.02ms1.01mpas_ocean.ConstructFaceLatLon.time_cartesian_averaging('120km')
3.94±0.02ms4.01±0.05ms1.02mpas_ocean.ConstructFaceLatLon.time_cartesian_averaging('480km')
3.42±0.01s3.44±0.01s1.01mpas_ocean.ConstructFaceLatLon.time_welzl('120km')
225±0.5ms224±0.6ms0.99mpas_ocean.ConstructFaceLatLon.time_welzl('480km')
18.2±0.04ms18.2±0.03ms1.00mpas_ocean.ConstructTreeStructures.time_ball_tree('120km')
1.04±0.02ms1.07±0.02ms1.03mpas_ocean.ConstructTreeStructures.time_ball_tree('480km')
10.6±0.03ms10.7±0.04ms1.02mpas_ocean.ConstructTreeStructures.time_kd_tree('120km')
690±4ms733±4ms1.06mpas_ocean.CrossSections.time_const_lat('120km', 1)
351±0.9ms371±4ms1.06mpas_ocean.CrossSections.time_const_lat('120km', 2)
184±0.3ms191±3ms1.04mpas_ocean.CrossSections.time_const_lat('120km', 4)
543±5ms564±4ms1.04mpas_ocean.CrossSections.time_const_lat('480km', 1)
275±2ms286±2ms1.04mpas_ocean.CrossSections.time_const_lat('480km', 2)
142±0.7ms146±2ms1.02mpas_ocean.CrossSections.time_const_lat('480km', 4)
24.5±0.1ms25.1±0.2ms1.02mpas_ocean.DualMesh.time_dual_mesh_construction('120km')
3.42±0.07ms3.37±0.07ms0.99mpas_ocean.DualMesh.time_dual_mesh_construction('480km')
954±6ms963±8ms1.01mpas_ocean.GeoDataFrame.time_to_geodataframe('120km', False)
50.4±0.8ms54.8±0.8ms1.09mpas_ocean.GeoDataFrame.time_to_geodataframe('120km', True)
83.5±0.7ms84.9±0.4ms1.02mpas_ocean.GeoDataFrame.time_to_geodataframe('480km', False)
5.41±0.1ms5.92±0.1ms1.09mpas_ocean.GeoDataFrame.time_to_geodataframe('480km', True)
454M454M1.00mpas_ocean.Gradient.peakmem_gradient('120km')
173±0.6ms177±1ms1.02mpas_ocean.Gradient.time_gradient('120km')
12.6±0.08ms12.5±0.1ms0.99mpas_ocean.Gradient.time_gradient('480km')
224±0.6μs225±2μs1.01mpas_ocean.HoleEdgeIndices.time_construct_hole_edge_indices('120km')
132±0.3μs133±0.6μs1.01mpas_ocean.HoleEdgeIndices.time_construct_hole_edge_indices('480km')
403M403M1.00mpas_ocean.Integrate.peakmem_integrate('120km')
382M383M1.00mpas_ocean.Integrate.peakmem_integrate('480km')
214±2μs218±2μs1.02mpas_ocean.Integrate.time_integrate('120km')
197±2μs202±2μs1.02mpas_ocean.Integrate.time_integrate('480km')
132±1ms135±0.8ms1.02mpas_ocean.MatplotlibConversion.time_dataarray_to_polycollection('120km', 'exclude')
127±1ms132±2ms1.05mpas_ocean.MatplotlibConversion.time_dataarray_to_polycollection('120km', 'include')
129±1ms131±1ms1.01mpas_ocean.MatplotlibConversion.time_dataarray_to_polycollection('120km', 'split')
10.2±0.2ms11.1±0.6ms1.09mpas_ocean.MatplotlibConversion.time_dataarray_to_polycollection('480km', 'exclude')
10.2±0.1ms10.6±0.1ms1.04mpas_ocean.MatplotlibConversion.time_dataarray_to_polycollection('480km', 'include')
10.1±0.2ms10.6±0.1ms1.06mpas_ocean.MatplotlibConversion.time_dataarray_to_polycollection('480km', 'split')
351±1μs355±1μs1.01mpas_ocean.PointInPolygon.time_face_search_lonlat('120km')
355±2μs357±2μs1.00mpas_ocean.PointInPolygon.time_face_search_lonlat('480km')
334±1μs336±2μs1.01mpas_ocean.PointInPolygon.time_face_search_xyz('120km')
338±1μs341±2μs1.01mpas_ocean.PointInPolygon.time_face_search_xyz('480km')
240±1ms248±0.8ms1.03mpas_ocean.RemapDownsample.time_bilinear_remapping
296±2ms302±2ms1.02mpas_ocean.RemapDownsample.time_inverse_distance_weighted_remapping
4.74±0.03ms4.94±0.2ms1.04mpas_ocean.RemapDownsample.time_nearest_neighbor_remapping
1.43±0s1.45±0.01s1.02mpas_ocean.RemapUpsample.time_bilinear_remapping
36.1±0.5ms37.7±0.5ms1.05mpas_ocean.RemapUpsample.time_inverse_distance_weighted_remapping
9.45±0.2ms9.70±0.2ms1.03mpas_ocean.RemapUpsample.time_nearest_neighbor_remapping
29.3±0.5ms29.9±1ms1.02mpas_ocean.ZonalAverage.time_zonal_average('120km')
6.51±0.02ms7.10±0.2ms1.09mpas_ocean.ZonalAverage.time_zonal_average('480km')
379M378M1.00quad_hexagon.QuadHexagon.peakmem_open_dataset
377M377M1.00quad_hexagon.QuadHexagon.peakmem_open_grid
7.00±0.2ms7.40±0.3ms1.06quad_hexagon.QuadHexagon.time_open_dataset
5.87±0.1ms6.59±0.3ms~1.12quad_hexagon.QuadHexagon.time_open_grid

Benchmarks that have got worse:

ChangeBefore [7c9b487]After [b85be3a]RatioBenchmark (Parameter)
+703±20μs817±40μs1.16mpas_ocean.ConstructTreeStructures.time_kd_tree('480km')

Sevans711
Sevans711 previously requested changes Jul 9, 2026

@Sevans711Sevans711 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good, though I would maybe suggest tiny changes for readability:

  • rename all_paths_exist to all_dyamond_paths_exist or something that suggests only glade/dyamond paths are being checked.
  • (less important) instead of looping with something like: flag = flag and check, might be more readable to do, e.g.: all(os.path.exists(file_path) for file_path in dyamond_path_dict.values())

@cmdupuis3

Copy link
Copy Markdown
CollaboratorAuthor

Maybe, I was trying to mirror what's in the dyamond benchmarks since it's a copy-paste job, so If we change one we should maybe change both.

@Sevans711
Sevans711 self-requested a review July 9, 2026 17:00
@Sevans711
Sevans711 dismissed their stale reviewJuly 9, 2026 17:02

Only requested style changes, and those changes deviate from the existing style in other dyamond tests.

@Sevans711Sevans711 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good, my only previous comments were about style, but this style actually matches the existing style from benchmarks/mpas_dyamond.py

@cmdupuis3
cmdupuis3 merged commit 5154420 into UXARRAY:mainJul 9, 2026
13 checks passed
@github-project-automationgithub-project-automationBot moved this from 👀 In review to ✅ Done in UXarray DevelopmentJul 9, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

benchmarkingRelated to benchmarks, memory usage, and/or time profilingrun-benchmarkRun ASV benchmark workflow

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

3 participants

@cmdupuis3@Sevans711@rajeeja