-
Notifications
You must be signed in to change notification settings - Fork 18
Wu/reverse accumulate #2531
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
Wu/reverse accumulate #2531
Changes from 4 commits
695823e
a9b4c26
15a6209
5b131e4
06e0761
979f527
7ee6dc2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -267,14 +267,15 @@ function column_accumulate!( | |
| input::Union{Fields.Field, PointwiseOrColumnwiseBroadcasted}; | ||
| init = UnspecifiedInit(), | ||
| transform::T = identity, | ||
| reverse::Bool = false, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment here about |
||
| ) where {F, T} | ||
| device = ClimaComms.device(output) | ||
| space = axes(input) | ||
| init == UnspecifiedInit() && | ||
| Spaces.staggering(space) == Spaces.CellCenter() && | ||
| Spaces.staggering(axes(output)) == Spaces.CellFace() && | ||
| error("init must be specified for center-to-face accumulation") | ||
| column_accumulate_device!(device, f, transform, output, input, init, space) | ||
| column_accumulate_device!(device, f, transform, output, input, init, space, reverse) | ||
| end | ||
|
|
||
| function column_accumulate_device!( | ||
|
|
@@ -285,11 +286,12 @@ function column_accumulate_device!( | |
| input, | ||
| init, | ||
| space, | ||
| reverse, | ||
| ) where {F, T} | ||
| mask = Spaces.get_mask(space) | ||
| if space isa Spaces.FiniteDifferenceSpace | ||
| @assert mask isa DataLayouts.NoMask | ||
| single_column_accumulate!(f, transform, output, input, init, space) | ||
| single_column_accumulate!(f, transform, output, input, init, space, reverse) | ||
| else | ||
| Fields.bycolumn(space) do colidx | ||
| I = Fields.universal_index(colidx) | ||
|
|
@@ -301,6 +303,7 @@ function column_accumulate_device!( | |
| input[colidx], | ||
| init, | ||
| space[colidx], | ||
| reverse, | ||
| ) | ||
| end | ||
| end | ||
|
|
@@ -315,6 +318,7 @@ function single_column_accumulate!( | |
| _input, | ||
| init, | ||
| space, | ||
| reverse, | ||
| ) where {F, T} | ||
| device = ClimaComms.device(space) | ||
| first_level = left_idx(space) | ||
|
|
@@ -337,7 +341,12 @@ function single_column_accumulate!( | |
| @inbounds if !isnothing(init_output_level) | ||
| Fields.level(output, init_output_level)[] = transform(accumulated_value) | ||
| end | ||
| @inbounds for level in next_level:last_level | ||
| indices = if reverse | ||
| last_level:-1:next_level | ||
| else | ||
| next_level:last_level | ||
| end | ||
| @inbounds for level in indices | ||
| accumulated_value = | ||
| f(accumulated_value, get_level_value(space, _input, level)) | ||
| output_level = | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -106,9 +106,12 @@ end | |
| function test_column_reduce_and_accumulate!(center_space) | ||
| face_space = center_to_face_space(center_space) | ||
| ᶜwhole_number = ones(center_space) | ||
| ᶜwhole_number_reverse = ones(center_space) | ||
| column_accumulate!(+, ᶜwhole_number, ᶜwhole_number) # 1:Nv per column | ||
| column_accumulate!(+, ᶜwhole_number_reverse, ᶜwhole_number_reverse; reverse = true) | ||
| ᶠwhole_number = ones(face_space) | ||
| column_accumulate!(+, ᶠwhole_number, ᶠwhole_number) # 1:(Nv + 1) per column | ||
| ᶠwhole_number_reverse = ones(face_space) | ||
| column_accumulate!(+, ᶠwhole_number_reverse, ᶠwhole_number_reverse; reverse = true) # 1:(Nv + 1) per column | ||
|
|
||
| safe_binomial(n, k) = binomial(Int32(n), Int32(k)) # GPU-compatible binomial | ||
|
|
||
|
|
@@ -139,19 +142,26 @@ function test_column_reduce_and_accumulate!(center_space) | |
|
|
||
| ᶜoutput = similar(ᶜwhole_number) | ||
| ᶠoutput = similar(ᶠwhole_number) | ||
| for (input, output, reference_output) in ( | ||
| (ᶜwhole_number, ᶜoutput, motzkin_number.(ᶜwhole_number)), | ||
| (ᶠwhole_number, ᶠoutput, motzkin_number.(ᶠwhole_number)), | ||
| (ᶠwhole_number, ᶜoutput, motzkin_number.(ᶜwhole_number .+ 1)), | ||
| (ᶜwhole_number, ᶠoutput, motzkin_number.(ᶠwhole_number .- 1)), | ||
| for (input, output, reference_output, reverse) in ( | ||
| (ᶜwhole_number, ᶜoutput, motzkin_number.(ᶜwhole_number), false), | ||
| (ᶠwhole_number, ᶠoutput, motzkin_number.(ᶠwhole_number), false), | ||
| (ᶠwhole_number, ᶜoutput, motzkin_number.(ᶜwhole_number .+ 1), false), | ||
| (ᶜwhole_number, ᶠoutput, motzkin_number.(ᶠwhole_number .- 1), false), | ||
| (ᶜwhole_number_reverse, ᶜoutput, motzkin_number.(ᶜwhole_number_reverse), true), | ||
| (ᶠwhole_number_reverse, ᶠoutput, motzkin_number.(ᶠwhole_number_reverse), true), | ||
| (ᶠwhole_number_reverse, ᶜoutput, motzkin_number.(ᶜwhole_number_reverse .+ 1), true), | ||
| (ᶜwhole_number_reverse, ᶠoutput, motzkin_number.(ᶠwhole_number_reverse .- 1), true), | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The tests looks adequate, but I'm not sure that they pass in the current state.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I ran the tests locally and they all passed. |
||
| ) | ||
|
|
||
| set_output! = | ||
| () -> column_accumulate!(f, output, input; init, transform) | ||
| () -> column_accumulate!(f, output, input; init, transform, reverse) | ||
| set_output!() | ||
| @test output == reference_output | ||
| @test_opt ignored_modules = CUDA_FRAMES set_output!() | ||
| test_allocs(@allocated set_output!()) | ||
| end | ||
|
|
||
|
|
||
| end | ||
|
|
||
| function test_fubinis_theorem(space) | ||
|
|
||
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.
I think the
reduceversion of this above requires the same change