| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
There was a problem hiding this comment.
Thanks for that, looks sane and definitely a bug. Some small comments
Sorry, something went wrong.
| """ | ||
| if not axes or axes == (None,): | ||
| new_order = tuple(range(self.ndim - 1, -1, -1)) | ||
| elif len(axes) == 1 and isinstance(axes[0], Iterable): |
There was a problem hiding this comment.
Can use devito.tools.as_tuple for these two cases
Sorry, something went wrong.
| new_order = tuple(ax % self.ndim for ax in new_order) | ||
|
|
||
| ret = super().transpose(*axes) | ||
| if isinstance(ret, Data): |
There was a problem hiding this comment.
Is this ever False?
Sorry, something went wrong.
| axis1 = axis1 % self.ndim | ||
| axis2 = axis2 % self.ndim | ||
| ret = super().swapaxes(axis1, axis2) | ||
| if isinstance(ret, Data): |
There was a problem hiding this comment.
same
Sorry, something went wrong.
| f = Function(name='f', grid=grid) | ||
| f.data[:] = np.arange(24).reshape((4, 6)).astype(np.float32) | ||
|
|
||
| ref = np.arange(24).reshape((4, 6)).astype(np.float32) |
There was a problem hiding this comment.
ref = np.array(f.data)
Sorry, something went wrong.
|
Thank you, please rebase with commit messages according to guidlines and this is gtg |
Sorry, something went wrong.
The numpy transpose primitive only permutes the buffer layout; the companion metadata that Data uses to translate global<->local indices under MPI (Data._decomposition and Data._modulo) was left in the pre-transpose axis order, so any indexing on a transposed Data either silently used the wrong decomposition slice or raised an IndexError that depends on the run's rank topology. Permute both metadata structures alongside the buffer in Data.transpose / Data.swapaxes via the same axes argument so they stay consistent with the new axis order, and add a regression test exercising 2D and 3D transposes against the equivalent numpy.ndarray.transpose result. Fixes devitocodes#2187
|
Done — rebased into a single compiler: tagged commit (420ecffe6). Should be ready for the CI gate now. |
Sorry, something went wrong.
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #2938 +/- ##
==========================================
- Coverage 83.35% 79.14% -4.21%
==========================================
Files 248 248
Lines 51905 51960 +55
Branches 4479 4480 +1
==========================================
- Hits 43265 41125 -2140
- Misses 7882 10026 +2144
- Partials 758 809 +51
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry.
|
Sorry, something went wrong.
There was a problem hiding this comment.
seems legit to me
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Fix: permute Data._decomposition / _modulo on transpose
Fixes #2187.
Problem
A Data view created via .T, transpose(...) or swapaxes(...)
inherited the source's _decomposition and _modulo tuples unchanged
through __array_finalize__ (the self.ndim == obj.ndim branch
explicitly does self._modulo = obj._modulo,
self._decomposition = obj._decomposition). Both tuples are indexed by
axis, so after the axes are permuted they point at the wrong
dimensions. A subsequent slice goes through __getitem__ →
_index_glb_to_loc(glb_idx), which uses the (stale) _decomposition
to translate the global slice into local-axis ranges, and silently
returns a wrong-shaped view.
The minimal example from #2187:
f.data[::2, ::2].T (slice then transpose) works because the slice
goes through __getitem__ first and the second-step transpose only
flips a 2-element metadata tuple whose entries are already aligned.
Why both transpose() and T need overriding
ndarray.T is a C-level shortcut: it does not dispatch through
Python's transpose method on a subclass. Verified directly:
So overriding transpose alone leaves .T buggy; overriding T alone
leaves explicit transpose / swapaxes calls buggy. Three thin
overrides are added on Data:
tuple/list/iterable, or per-arg individual axes), call
super().transpose, then permute _decomposition / _modulo into
the new axis order and refresh _is_distributed so it stays
consistent.
Verification
The two new tests in tests/test_data.py::TestDataBasic:
transpose((1, 0, 2)), swapaxes(0, 1), in both 2D and 3D, with
the NumPy reference array as the source of truth (shape and
values are compared).
_decomposition / _modulo end up in the new order (reverse for
.T, swap for swapaxes, explicit permutation for
transpose((1, 2, 0))).
Local run on main HEAD (no MPI installed; MPI-only tests pre-existing
failures are unchanged):
Scope
Just devito/data/data.py (production fix) and tests/test_data.py
(regression tests). No public API change — transpose / T /
swapaxes signatures are unchanged; behavior matches NumPy.