Repository navigation
Bug: structured data transpose changes face mapping in from_xarray #1828
Description
Version
Source main at 6d6c6dacd2472fda8c6a3f71af659e1eb1cbadc9; Python 3.12.14, NumPy 2.5.4, xarray 2026.9.0 on Linux.
How did you install UXarray?
Source
What happened?
Converting equivalent structured datasets with UxDataset.from_xarray(..., uxgrid=grid) associates different values with the same grid faces when the spatial dimension order is transposed. The conversion succeeds without a warning. This matters when normalizing structured member files into a shared unstructured representation for regional statistics or integration.
For the synthetic example below, (y, x) produces face values [0, 0, 0, 100, 100, 100], while the equivalent (x, y) input produces [0, 100, 0, 100, 0, 100]. Both retain exactly the same Grid object and its face connectivity. The integral changes from 67.08123535225863 to 79.37563993526874 for this example; this is not a general error bound.
What did you expect to happen?
Transposing named input dimensions should preserve the physical field and its association with grid faces, so both mapped values and integration results should agree.
Can you provide a MCVE to repoduce the bug?
import numpy as np
import xarray as xr
import uxarray as ux
original = xr.Dataset(
{"temperature": (("y", "x"), [[0., 0., 0.], [100., 100., 100.]])},
coords={"x": [0., 20., 40.], "y": [-10., 50.]},
)
transposed = original.transpose("x", "y")
xr.testing.assert_identical(original, transposed.transpose("y", "x"))
grid = ux.Grid.from_structured(lon=original.x, lat=original.y)
a = ux.UxDataset.from_xarray(original, uxgrid=grid)
b = ux.UxDataset.from_xarray(transposed, uxgrid=grid)
assert a.uxgrid is b.uxgrid is grid
print(a.temperature.values) # [0., 0., 0., 100., 100., 100.]
print(b.temperature.values) # [0., 100., 0., 100., 0., 100.]
print(float(a.temperature.integrate())) # 67.08123535225863
print(float(b.temperature.integrate())) # 79.37563993526874
np.testing.assert_array_equal(a.temperature.values, b.temperature.values)The last assertion fails. I also reproduced the mapping after writing the two synthetic members as separate NetCDF files and reading them through xarray. Explicitly transposing the source back to (..., "y", "x") before conversion restores the correct values and integral.
Investigation and scope
_map_dims_to_ugrid chooses its stacking order from a data variable's final dimension, while _read_structured_grid constructs face connectivity in latitude-major, longitude-minor order. The existing structured mapping regression test also documents face k as (k // n_lon, k % n_lon).
This reproduction uses the existing from_xarray API on main, without the changes in #1827. It does not call weighted_mean, so it is distinct from #1800's reduction-axis issue. No implementation changes are included here. I would be happy to work on a focused fix and regression tests if this can be assigned to me, following the contribution guide.
AI assistance: OpenAI Codex assisted with investigation and synthetic reproduction; the recorded results were actually executed against the pinned source.
Metadata
Metadata
Labels
Type
Fields
Priority
Projects
- Status
Activity
Sevans711 commentedon Oct 19, 2026
Hi @sunxiuguo, thank you for reporting this bug! Also, please feel free to start work on this, I have added you as an assignee, and thank you for your interest to solve it! Although, before you go too deep into coding up a solution, could you share your thought processes / plans here?
I looked into this a bit more and it is remains unclear to me whether
UxDataset.from_xarray()is actually intended to support inputs with dimensions like "x" and "y". The examples online (e.g., https://uxarray.readthedocs.io/en/latest/user-guide/structured.html) seem to useUxDataset.from_structured()when the xarray dataset has "lon" and "lat" values, andUxDataset.from_xarray()only when the xarray dataset already has a grid dimension like "n_face". I suspect various uxarray devs may have different opinions on the "best" way to proceed here; I can already think of a few conflicting options that each sound reasonable, such as (1) support the use case you describe or (2) intentionally crash upon callingUxDataset.from_xarray()in this case, but with a helpful error message suggesting to useUxDataset.from_structured()instead. Either way, I think improving the docstrings on these methods will be quite helpful.If you are able to share your thoughts / plans here, my hope is that that could lead to some helpful discussions and allow everyone to get on the same page about these design decisions before you put too much effort towards an implementation!
sunxiuguo commentedon Oct 21, 2026
Thanks for the clarification. I agree we should settle the API contract before implementing a fix.
In the current source at 65e9c4a, the structured guide explicitly demonstrates Grid.from_structured(lon=..., lat=...) followed by UxDataset.from_xarray(ds, uxgrid=grid). The existing #1410 regression checks that path, whereas the from_xarray docstring describes unstructured inputs. My x/y example uses dimensions recorded by an explicitly supplied structured Grid; it does not ask UXarray to infer arbitrary coordinate meanings.
I would prefer retaining that documented path and flattening according to the Grid's face order, independently of variable dimension order, with clearer docstrings. Both constructors call the shared mapping helper, so both need consideration.
If structured inputs should instead be rejected by from_xarray, the guide and #1410 regression would need updating too. What would be the supported replacement for reusing an existing Grid? from_structured currently constructs one internally.
After agreement, I propose synthetic regressions for canonical/transposed and mixed variable orders, an extra time dimension, lazy chunks, already-n_face inputs, face-value alignment after selection, and consistent area integrals. The reported six-cell reproduction ran on 6d6c6da; I have rechecked the current source and documentation, but have not rerun that reproduction or this proposed test matrix on current main.
Which contract would you prefer? AI-assisted analysis and drafting: ChatGPT. No implementation for this issue has been submitted; this face-order decision is separate from #1827's Grid-reuse fixes.
Sevans711 commentedon Oct 21, 2026
Ah, thank you for clarifying these details, this helped me get a much better understanding of the problem. First, clarifying briefly clarifying one point: while the structured guide demonstrates
Grid.from_structured(lon=..., lat=...)followed byUxDataset.from_xarray(ds, uxgrid=grid), thedsused there has dimension "n_face" already, so it wasn't immediately clear to me whether the automatic-stacking behavior was intentional. The regression test you linked shows that this behavior is indeed intention, though.Looking into it a bit further, I now personally agree that the bug to be fixed here is indeed in
_map_dims_to_ugridin theif grid.source_grid_spec == "Structured"branch. It should simply stack asds = ds.stack(n_face=_source_dims_dict["n_face"]), rather than inferring dims order from the order of dims in the last data_var which contains bothlon_nameandlat_name.I emphasized "last" here because I realized the current version of the code also depends on the data_vars order! For example:
I think it's fair to say that the fix here is less ambiguous than I originally thought. Specifically, I think the fix is:
_map_dims_to_ugridshould stack asds = ds.stack(n_face=_source_dims_dict["n_face"])instead of checking data vars' dims order.A bonus thing to look into might be the UxDataArray.from_xarray method, which currently has issues too:
ux.UxDataArray.from_xarray(ds0['xfirst'], uxgrid=grid0)makesAttributeError: 'DataArray' object has no attribute 'data_vars'. Its docstring also starts by promising to return a UxDataset, but ends by promising to return a UxDataArray. (The fix here would be to clarify the output will be a UxDataArray, and fix the code to actually work properly.)Although, this isn't in scope for the original issue you mentioned, it's just something extra I noticed which happens to be related; it could either be solved here or moved to a separate issue/PR (I'd be happy to post that one if you want), whichever you prefer!
Sevans711 commentedon Oct 21, 2026
All that said, I think we're on the same page now? If you agree with what I said above regarding the fix here, I think it would be totally fine to move forward with it at this point, because I think it is well-defined and the fix is clear. If we're not fully on the same page let's continue to discuss here first, though!
sunxiuguo commentedon Oct 21, 2026
Thanks, I agree with removing the data-variable-order inference and clarifying UxDataset.from_xarray, while keeping the DataArray issue separate.
One implementation detail needs checking: on current main 65e9c4a, the stored tuple is (lon_name, lat_name), but face construction is latitude-major, longitude-minor, as the #1410 regression also asserts. So I would retain the tuple contract and use ds.stack(n_face=(lat_name, lon_name)), rather than passing that stored tuple directly.
I tested this with a synthetic 2-by-3 grid at latitudes -10 and 50 degrees and values [[0,20,40],[100,120,140]]. Direct tuple stacking produces [0,100,20,120,40,140]; the geometry-aligned order is [0,20,40,100,120,140]. Comparing only the two reordered datasets would miss this error because both would be consistently wrong.
The new 27-case regression matrix has 15 failures on unchanged main, 26 with direct tuple stacking, and none with (lat_name, lon_name). It checks both Dataset constructors, variable insertion order, time/spatial axis permutations, lazy chunks, synthetic NetCDF reopening, face selection, and integrals; this is not a full-suite result.
Does retaining the current tuple and explicitly using its latitude-then-longitude order match the intended fix? The candidate is local only; I have not submitted a PR for this issue. AI assistance: OpenAI Codex.
Sevans711 commentedon Oct 21, 2026
Oh, great catch. The fix here should use whichever order does not break the #1410 regression test. I agree that the
ds.stack(n_face=(lat_name, lon_name))order seems to make more sense than theds = ds.stack(n_face=_source_dims_dict["n_face"])order. Please feel free to move forward with a PR with that in mind! And, thank you again for looking into this in detail, and for your work on it so far!