Skip to content

Fix structured from_xarray n_face mapping (#1410)#1569

Open
rajeeja wants to merge 3 commits into
mainfrom
rajeeja/fix_1410
Open

Fix structured from_xarray n_face mapping (#1410)#1569
rajeeja wants to merge 3 commits into
mainfrom
rajeeja/fix_1410

Conversation

@rajeeja

@rajeeja rajeeja commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Fixes #1410 and the three bugs behind it:

  1. from_structured(lon=, lat=) used lowercase "structured", so the structured-flatten branch never ran.
  2. It passed source_dims_dict=None, so (lon, lat) → n_face was never recorded.
  3. from_dataset hard-coded source_dims_dict = {}, dropping any passed mapping.

rajeeja added 2 commits July 13, 2026 15:50
Grid.from_structured(lon=, lat=) recorded no source-dim mapping and used a
lowercase "structured" spec, so UxDataset.from_xarray could not flatten
(lon, lat) data variables onto n_face, leaving them unmapped and breaking
subsetting. Capture the lon/lat dim names, use the canonical "Structured"
spec, and honor a passed source_dims_dict in from_dataset. Closes #1410.
@review-notebook-app

Copy link
Copy Markdown

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@erogluorhan erogluorhan added the run-benchmark Run ASV benchmark workflow label Jul 15, 2026
@github-actions

Copy link
Copy Markdown

ASV Benchmarking

Benchmark Comparison Results

Benchmarks that have improved:

Change Before [736bc03] After [f3e9ca5] Ratio Benchmark (Parameter)
- 579M 391M 0.67 face_bounds.FaceBounds.peakmem_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/ugrid/geoflow-small/grid.nc'))
- 700M 390M 0.56 face_bounds.FaceBounds.peakmem_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/ugrid/quad-hexagon/grid.nc'))
- 498M 384M 0.77 mpas_ocean.Gradient.peakmem_gradient('480km')

Benchmarks that have stayed the same:

Change Before [736bc03] After [f3e9ca5] Ratio Benchmark (Parameter)
9.88±0.1μs 9.81±0.06μs 0.99 bench_connectivity.Connectivity.time_edge_face('120km')
10.2±0.1μs 10.1±0.1μs 0.99 bench_connectivity.Connectivity.time_edge_face('480km')
9.95±0.04μs 9.90±0.1μs 0.99 bench_connectivity.Connectivity.time_edge_node('120km')
10.1±0.06μs 10.3±0.1μs 1.02 bench_connectivity.Connectivity.time_edge_node('480km')
9.88±0.06μs 9.94±0.1μs 1.01 bench_connectivity.Connectivity.time_face_edge('120km')
10.2±0.09μs 10.6±0.06μs 1.03 bench_connectivity.Connectivity.time_face_edge('480km')
9.91±0.08μs 9.89±0.05μs 1 bench_connectivity.Connectivity.time_face_face('120km')
10.1±0.1μs 10.1±0.06μs 1 bench_connectivity.Connectivity.time_face_face('480km')
20.8±0.2μs 20.0±0.09μs 0.96 bench_connectivity.Connectivity.time_face_node('120km')
20.3±0.2μs 21.2±0.8μs 1.04 bench_connectivity.Connectivity.time_face_node('480km')
9.92±0.04μs 9.85±0.02μs 0.99 bench_connectivity.Connectivity.time_node_edge('120km')
10.4±0.4μs 10.1±0.1μs 0.97 bench_connectivity.Connectivity.time_node_edge('480km')
9.96±0.2μs 9.75±0.07μs 0.98 bench_connectivity.Connectivity.time_node_face('120km')
10.3±0.09μs 10.1±0.03μs 0.98 bench_connectivity.Connectivity.time_node_face('480km')
389M 389M 1 face_bounds.FaceBounds.peakmem_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/mpas/QU/oQU480.231010.nc'))
421M 419M 0.99 face_bounds.FaceBounds.peakmem_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/scrip/outCSne8/outCSne8.nc'))
12.8±0.04ms 12.8±0.02ms 1 face_bounds.FaceBounds.time_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/mpas/QU/oQU480.231010.nc'))
3.27±0.02ms 3.29±0.08ms 1 face_bounds.FaceBounds.time_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/scrip/outCSne8/outCSne8.nc'))
17.4±0.1ms 17.4±0.08ms 1 face_bounds.FaceBounds.time_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/ugrid/geoflow-small/grid.nc'))
1.99±0.04ms 1.99±0.04ms 1 face_bounds.FaceBounds.time_face_bounds(PosixPath('/home/runner/work/uxarray/uxarray/test/meshfiles/ugrid/quad-hexagon/grid.nc'))
998±8ms 990±7ms 0.99 import.Imports.timeraw_import_uxarray
906±30ns 904±4ns 1 mpas_ocean.CheckNorm.time_check_norm('120km')
844±3ns 855±2ns 1.01 mpas_ocean.CheckNorm.time_check_norm('480km')
721±2ms 727±20ms 1.01 mpas_ocean.ConnectivityConstruction.time_face_face_connectivity('120km')
45.1±0.2ms 45.0±0.3ms 1 mpas_ocean.ConnectivityConstruction.time_face_face_connectivity('480km')
621±7μs 623±6μs 1 mpas_ocean.ConnectivityConstruction.time_n_nodes_per_face('120km')
556±7μs 553±7μs 1 mpas_ocean.ConnectivityConstruction.time_n_nodes_per_face('480km')
5.04±0.03ms 5.06±0.01ms 1.01 mpas_ocean.ConstructFaceLatLon.time_cartesian_averaging('120km')
3.55±0.02ms 3.52±0.02ms 0.99 mpas_ocean.ConstructFaceLatLon.time_cartesian_averaging('480km')
3.30±0.03s 3.33±0.02s 1.01 mpas_ocean.ConstructFaceLatLon.time_welzl('120km')
215±2ms 215±0.6ms 1 mpas_ocean.ConstructFaceLatLon.time_welzl('480km')
19.7±0.03ms 19.8±0.1ms 1.01 mpas_ocean.ConstructTreeStructures.time_ball_tree('120km')
1.12±0ms 1.11±0.01ms 0.99 mpas_ocean.ConstructTreeStructures.time_ball_tree('480km')
10.6±0.02ms 10.6±0.02ms 1 mpas_ocean.ConstructTreeStructures.time_kd_tree('120km')
650±6ms 652±8ms 1 mpas_ocean.CrossSections.time_const_lat('120km', 1)
330±1ms 331±0.8ms 1 mpas_ocean.CrossSections.time_const_lat('120km', 2)
170±0.5ms 171±2ms 1.01 mpas_ocean.CrossSections.time_const_lat('120km', 4)
487±0.5ms 488±3ms 1 mpas_ocean.CrossSections.time_const_lat('480km', 1)
244±0.8ms 247±2ms 1.01 mpas_ocean.CrossSections.time_const_lat('480km', 2)
126±0.4ms 128±0.5ms 1.01 mpas_ocean.CrossSections.time_const_lat('480km', 4)
22.0±0.02ms 22.3±0.3ms 1.01 mpas_ocean.DualMesh.time_dual_mesh_construction('120km')
2.57±0.01ms 2.64±0.09ms 1.03 mpas_ocean.DualMesh.time_dual_mesh_construction('480km')
838±5ms 845±6ms 1.01 mpas_ocean.GeoDataFrame.time_to_geodataframe('120km', False)
49.6±0.7ms 49.6±0.7ms 1 mpas_ocean.GeoDataFrame.time_to_geodataframe('120km', True)
73.4±0.9ms 72.9±0.3ms 0.99 mpas_ocean.GeoDataFrame.time_to_geodataframe('480km', False)
5.70±0.2ms 5.42±0.1ms 0.95 mpas_ocean.GeoDataFrame.time_to_geodataframe('480km', True)
404M 404M 1 mpas_ocean.Gradient.peakmem_gradient('120km')
166±0.7ms 168±4ms 1.01 mpas_ocean.Gradient.time_gradient('120km')
11.4±0.2ms 11.3±0.02ms 0.99 mpas_ocean.Gradient.time_gradient('480km')
190±4μs 189±1μs 1 mpas_ocean.HoleEdgeIndices.time_construct_hole_edge_indices('120km')
87.8±0.4μs 86.6±1μs 0.99 mpas_ocean.HoleEdgeIndices.time_construct_hole_edge_indices('480km')
351M 351M 1 mpas_ocean.Integrate.peakmem_integrate('120km')
331M 330M 1 mpas_ocean.Integrate.peakmem_integrate('480km')
180±1μs 181±1μs 1 mpas_ocean.Integrate.time_integrate('120km')
156±1μs 158±0.6μs 1.01 mpas_ocean.Integrate.time_integrate('480km')
188±2ms 188±2ms 1 mpas_ocean.MatplotlibConversion.time_dataarray_to_polycollection('120km', 'exclude')
190±3ms 192±3ms 1.01 mpas_ocean.MatplotlibConversion.time_dataarray_to_polycollection('120km', 'include')
188±1ms 192±2ms 1.02 mpas_ocean.MatplotlibConversion.time_dataarray_to_polycollection('120km', 'split')
14.0±0.2ms 13.9±0.1ms 0.99 mpas_ocean.MatplotlibConversion.time_dataarray_to_polycollection('480km', 'exclude')
13.9±0.04ms 13.7±0.04ms 0.99 mpas_ocean.MatplotlibConversion.time_dataarray_to_polycollection('480km', 'include')
14.0±0.1ms 13.6±0.03ms 0.97 mpas_ocean.MatplotlibConversion.time_dataarray_to_polycollection('480km', 'split')
281±2μs 284±3μs 1.01 mpas_ocean.PointInPolygon.time_face_search_lonlat('120km')
286±1μs 281±3μs 0.98 mpas_ocean.PointInPolygon.time_face_search_lonlat('480km')
266±1μs 263±1μs 0.99 mpas_ocean.PointInPolygon.time_face_search_xyz('120km')
268±1μs 267±1μs 1 mpas_ocean.PointInPolygon.time_face_search_xyz('480km')
212±1ms 212±0.5ms 1 mpas_ocean.RemapDownsample.time_bilinear_remapping
228±2ms 230±2ms 1.01 mpas_ocean.RemapDownsample.time_inverse_distance_weighted_remapping
4.25±0.05ms 4.26±0.01ms 1 mpas_ocean.RemapDownsample.time_nearest_neighbor_remapping
1.18±0.01s 1.17±0s 1 mpas_ocean.RemapUpsample.time_bilinear_remapping
35.5±0.2ms 36.7±0.6ms 1.03 mpas_ocean.RemapUpsample.time_inverse_distance_weighted_remapping
8.83±0.2ms 9.05±0.09ms 1.03 mpas_ocean.RemapUpsample.time_nearest_neighbor_remapping
29.3±0.4ms 28.6±0.2ms 0.98 mpas_ocean.ZonalAverage.time_zonal_average('120km')
6.13±0.06ms 5.90±0.02ms 0.96 mpas_ocean.ZonalAverage.time_zonal_average('480km')
328M 328M 1 quad_hexagon.QuadHexagon.peakmem_open_dataset
325M 326M 1 quad_hexagon.QuadHexagon.peakmem_open_grid
6.59±0.09ms 6.49±0.09ms 0.99 quad_hexagon.QuadHexagon.time_open_dataset
5.55±0.1ms 5.49±0.06ms 0.99 quad_hexagon.QuadHexagon.time_open_grid

Benchmarks that have got worse:

Change Before [736bc03] After [f3e9ca5] Ratio Benchmark (Parameter)
+ 749±2μs 828±50μs 1.11 mpas_ocean.ConstructTreeStructures.time_kd_tree('480km')

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

Development

Successfully merging this pull request may close these issues.

When generalizing a structured dataset with UxDataset.from_structured, return information about faces

2 participants