Repository navigation
Reduce over the interior of fields - #6228
Merged
Merged
Conversation
GPUArrays 12 reduces through AcceleratedKernels, which takes a linear view of the source array (`@view src[begin:end]`), reshapes it, and adapts the result for the kernel. Reductions without a condition passed the `Field` itself, and a `Field` doesn't support any of this: `lastindex` is the last index of its halo'd data, a one-index `view` is an x-slice, and adapting it returns its data with halos, so the reshaped array no longer fits. Reductions now get the field's interior, a 1-based array of the field's own size, as the index reductions already do. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EtLmZUPY4g9hi8aJsM4Wrs
The size of an `AbstractField` ignored its indices while its axes honour them, so for an operation on a windowed field `length` and `axes` disagreed, and reshaping it, as GPUArrays 12 reductions do, failed. The size now honours the indices, like a `Field`'s. Operations also drop the indices of their fields when adapted for a kernel, which changes their size and breaks the reshape built around them, so reductions of windowed operations, `dot` included, go through a view of the window, which keeps its size when adapted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EtLmZUPY4g9hi8aJsM4Wrs
A `TimeDerivative` adapts to its result, a `Field`, and so to the result's data with halos, which is larger than the derivative. Like a bare field, it broke GPUArrays 12 reductions, which reshape the source and then adapt it, e.g. when averaging a time derivative for output. Reductions now see the interior of its result. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EtLmZUPY4g9hi8aJsM4Wrs
giordano
added this pull request to stack #6231
October 11, 2026 08:01
navidcy
approved these changes
Oct 11, 2026
This was referenced Oct 13, 2026
xkykai
pushed a commit
that referenced
this pull request
Oct 13, 2026
Fields and operations drop their indices when adapted for a kernel, so since #6228 the size of an adapted operation indexed into nothing, breaking GPU index reductions such as argmin on a ConditionalOperation. Use axes_indices, as axes already does. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Guess what? Yes, this is also taken out of #5799