Skip to content

SCH-1073: Add face-centered subset support#5

Merged
kjnam merged 4 commits into
mainfrom
SCH-1073-add-face-centered-subset
Apr 28, 2026
Merged

SCH-1073: Add face-centered subset support#5
kjnam merged 4 commits into
mainfrom
SCH-1073-add-face-centered-subset

Conversation

@aesgeorges
Copy link
Copy Markdown
Contributor

SCH-1073: Add face-centered subset support

Summary

Adds support for subsetting face-centered (element) data by computing and assigning z-coordinates at edges and faces, and fixes an issue with sgrid_isel that blocked element-based subsetting.

Changes

  • Z-coordinate computation: suxarray/io/_schismgrid.py

    • Added _assign_z_coords to derive 1D static (node_z, edge_z, face_z) and 3D time-varying (node_z_3d, edge_z_3d, face_z_3d) z-coordinates from node-level zCoordinates.
    • Added _calculate_edge_z — averages node z-coordinates for each edge.
    • Added _calculate_face_z — averages node z-coordinates for each face, handling triangular and quad elements.
    • Integrated assignment into _read_schism_grid so z-coordinates are assigned when z-coordinate files are provided.
  • Bug fix: suxarray/grid/grid.py

    • sgrid_isel: now excludes inverse_indices along with grid dimensions to avoid invalid xarray-dimension errors during subsetting.
  • Tests & fixtures:

    • tests/testfixtures.py: added sxds_element_test fixture loading face-centered test data (out2d_alt_*.nc, zCoordinates_alt_*.nc, verticalVelAtElement_*.nc).
    • tests/test_suxarray.py: added test_subset_elements_bounding_polygon to verify bounding-box subsetting on element-centered data.
    • Added corresponding test data files under tests/testdata/.

Notes

  • Edge/face z-coordinate calculations are currently using NumPy; TODOs added to make them dask-compatible for large datasets.
  • Face z-coordinate averaging for quad elements uses a simple mean; consider a more rigorous center calculation in a follow-up.

@aesgeorges aesgeorges requested a review from kjnam February 23, 2026 18:35
Copy link
Copy Markdown
Member

@kjnam kjnam left a comment

Choose a reason for hiding this comment

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

Alex tested with a full Bay-Delta data and reported the change worked fine.

@kjnam kjnam merged commit 83c2b04 into main Apr 28, 2026
2 checks passed
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.

2 participants