Skip to content

Conversation

@rajeeja
Copy link
Contributor

@rajeeja rajeeja commented Dec 8, 2025

Fixes #98 This PR implements the cumulative_integrate method on UxDataArray. This is a wrapper around the upstream xarray.DataArray.cumulative_integrate method, enhanced with uxarray "grid-awareness" to support unstructured grid workflows, specifically for Cross-Sections (Great Circle Arcs). The original issue was created in 2022 - since then, xarray has implemented a robust cumulative_integrate method (using the Trapezoidal rule). This PR exposes that functionality directly in uxarray, closing the loop on 1D path analysis.

It must be noted that this is different from integrate:

uxds.integrate(): Calculates Total area/volume (Definite Integral) using grid geometry (Jacobians/Face Areas).

uxds.cumulative_integrate(): Calculates Running accumulation (Indefinite Integral) along a specific coordinate path.

@rajeeja rajeeja requested a review from erogluorhan December 8, 2025 13:36
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.

Thanks for putting this together! I think I am envisioning the cumulative_integrate user API a little differently than how it currently wraps the xarray one, i.e. actually a bit closer to the way the test case you've written around cross_sections handles it.

In other words, having the function signature similar to its xarray counterpart may not be intuitive since coords maybe quite different in the unstructured grids' case (e.g. node_lon, face_lat, etc. in contrast to structured lat/lon in xarray's case). Instead, shouldn't it be something like this (which is similar to our cross-sections):

def cumulative_integrate(self, lat=lat)
def cumulative_integrate(self, lon=lon)
def cumulative_integrate(self, start=(...), end=(...))

which should under the hood call our cross-section on them and then xarray
s cumulative-integrate on that result, i.e. the way the current test-case is implemented.

Or, instead of these, just do:

def cumulative_integrate(self, direction=direction)

where direction can get values of either "lon" or "lat". Not calling it coord intentionally because there is no "lat", 'lon' coords in uxarray datarrays.

Thoughts?

@rajeeja
Copy link
Contributor Author

rajeeja commented Dec 13, 2025

Thanks for putting this together! I think I am envisioning the cumulative_integrate user API a little differently than how it currently wraps the xarray one, i.e. actually a bit closer to the way the test case you've written around cross_sections handles it.

In other words, having the function signature similar to its xarray counterpart may not be intuitive since coords maybe quite different in the unstructured grids' case (e.g. node_lon, face_lat, etc. in contrast to structured lat/lon in xarray's case). Instead, shouldn't it be something like this (which is similar to our cross-sections):

def cumulative_integrate(self, lat=lat) def cumulative_integrate(self, lon=lon) def cumulative_integrate(self, start=(...), end=(...))

which should under the hood call our cross-section on them and then xarray s cumulative-integrate on that result, i.e. the way the current test-case is implemented.

Or, instead of these, just do:

def cumulative_integrate(self, direction=direction)

where direction can get values of either "lon" or "lat". Not calling it coord intentionally because there is no "lat", 'lon' coords in uxarray datarrays.

Thoughts?

Consider a time‑series UxDataArray on faces with dims ("time", "n_face"), we may want cumulative precipitation along time. With the proposed lat/lon/start-end/direction signature, there’s no way to specify the time coord—forcing a cross_section... I think the current xarray-like coord=... preserves these valid uses (time, level, custom distance) without forcing a cross_section that doesn’t apply.

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.

Add integrate and some cumulative_integrate

3 participants