-
Notifications
You must be signed in to change notification settings - Fork 195
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adds support for using Base.mapreducedim!
with AbstractField and AbstractOperations
#1611
Conversation
Base.mapreducedim!
with AbstractField and AbstractOperations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like a very positive change
using Oceananigans.Fields: architecture, tracernames | ||
using Oceananigans.Architectures: device | ||
using Oceananigans.Utils: work_layout | ||
using Oceananigans.Utils: work_layout, new_data | ||
using Oceananigans.Grids: new_data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't know if it's relevant, just caught the fact that new_data
is being imported from two different modules. Is that correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not correct -- nice catch!
mean!(avg.data.parent, operand_sans_halos) | ||
|
||
avg .= 0 # should we have to do this? | ||
mean!(avg, avg.operand) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Much simpler!
…/Oceananigans.jl into glw/mapreduce-abstract-ops
This PR adds support for in-place reduction operations where the target is an
AbstractReducedField
and the source is either anAbstractDataField
,AbstractOperation
, or array of some kind. These work on the GPU. In the end the solution was simple since we now subtypeAbstractArray
; we only need to pass a view into the interior indices of the target toBase.mapreducedim!
(which on the GPU ends up atGPUArrays.mapreducedim!
).The result is that
AveragedField(op::AbstractOperation)
no longer needs to allocate memory for the three-dimensional result of computingop
. Instead,op
is reduced in a kernel. This is both faster (much much faster, I think --- though a benchmark is a good idea) and more memory efficient. It also greatly simplifiescompute!(field::AveragedField)
:So, resolves #1422.
I also took the liberty of resolving #1610 and nuking
interiorparent
...