-
Notifications
You must be signed in to change notification settings - Fork 274
Refactor fill_halo_regions!: Reduce boundary conditions slowdown
#4706
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
Merged
Merged
Changes from all commits
Commits
Show all changes
126 commits
Select commit
Hold shift + click to select a range
1a05c0b
start it
simone-silvestri 780060b
change
simone-silvestri e8125e1
add this
simone-silvestri 952bee4
also this goes away
simone-silvestri 6e3618c
all away
simone-silvestri 21fd637
let's go
simone-silvestri 44655da
go with it
simone-silvestri a061b7f
go for it
simone-silvestri f8c9dec
Update hydrostatic_free_surface_tendency_kernel_functions.jl
simone-silvestri 3f82063
remove number of lines
simone-silvestri 2d5cf2a
Merge branch 'ss/catke-for-rk3' of github.com:CliMA/Oceananigans.jl i…
simone-silvestri c711b81
bugfix
simone-silvestri 09850d0
make it work
simone-silvestri f1cfb4d
go like this
simone-silvestri 617c429
vestigial stuff
simone-silvestri 1a7d6b1
fix
simone-silvestri 7611c14
go ahead
simone-silvestri 2ff463c
let;s go
simone-silvestri 0ad96c0
fix a bit
simone-silvestri 51ea3d7
go ahead
simone-silvestri 9fa3da7
this should all work
simone-silvestri 347abf3
add a tuple
simone-silvestri 393d590
Merge branch 'main' into ss/fix-bc-slowdown
simone-silvestri 51322fd
test hypothesis
simone-silvestri 4b19e27
Merge branch 'ss/fix-bc-slowdown' of github.com:CliMA/Oceananigans.jl…
simone-silvestri 6738f32
fix
simone-silvestri 16b40b3
just remove this tupled fill halo regions
simone-silvestri 644b757
remove signed
simone-silvestri 3016b13
just remove tupled fill_halo_regions
simone-silvestri 8927c62
startg simplifying
simone-silvestri b238ab3
start precomputing kernels
simone-silvestri 16bd2cd
start with the simple ones
simone-silvestri df8369e
this works for one field
simone-silvestri 5627e1d
some refactoring
simone-silvestri bc910d0
more refactorinf
simone-silvestri 8b7ab6d
remove multi region for the moment
simone-silvestri 02a882b
ok, this starts to shape up
simone-silvestri 571c6b3
remove this for now
simone-silvestri 29458c8
Merge branch 'main' into ss/fix-bc-slowdown
simone-silvestri 70b49a5
go ahead
simone-silvestri 71bff55
Merge branch 'ss/fix-bc-slowdown' of github.com:CliMA/Oceananigans.jl…
simone-silvestri 9672b3c
simplify
simone-silvestri 33fbf2e
more changes
simone-silvestri baeef6f
more changes
simone-silvestri c127bae
more fixing
simone-silvestri d5b4d79
at the top level
simone-silvestri 2e93da2
remove unuset code
simone-silvestri 2868cd6
go ahead
simone-silvestri 47e1ea6
more chnages
simone-silvestri 10a709b
think about this later on
simone-silvestri 38a06c9
think about this later on
simone-silvestri aae4a30
more chnages
simone-silvestri db6031f
refactor
simone-silvestri f491030
remove the val
simone-silvestri 06c275b
tuple not a vector
simone-silvestri 41d0366
fix some typos
simone-silvestri 048d4b8
add also the polar BC
simone-silvestri 0655910
more fixing
simone-silvestri 52ec822
deal with open BC
simone-silvestri d0aaa89
change stuff
simone-silvestri e950713
add a reduced dimension
simone-silvestri fc75a05
all but multiregion tests should pass?
simone-silvestri 5246cdb
still multi-region to fix
simone-silvestri 722c768
dealt with open BCS
simone-silvestri 5a50c28
BCS integration works
simone-silvestri 09b72c0
had to change a bit the atol
simone-silvestri cd133fb
reset the bcs test
simone-silvestri 6ca2299
no need for reduced_dimensions no more
simone-silvestri 47fb6b0
distributed works, let's go!
simone-silvestri 1867523
distributed works
simone-silvestri ec18906
include multiregion
simone-silvestri 2fea58d
also multiregion works! Let's go!
simone-silvestri 4eaa117
fix also the tripolar grid
simone-silvestri bfdecd3
make sure we can fill ALL boundary conditions
simone-silvestri 9081415
typo
simone-silvestri 6f28c13
more fixes
simone-silvestri 8448f73
fix tripolar tests
simone-silvestri d418d52
remove vestigial comments
simone-silvestri f0a80ad
Update boundary_condition_ordering.jl
simone-silvestri d4aded5
Update fill_halo_kernels.jl
simone-silvestri 47aa233
Update fill_halo_regions_open.jl
simone-silvestri 10a62af
Update field.jl
simone-silvestri 921d8a7
Update cubed_sphere_boundary_conditions.jl
simone-silvestri c1908b7
recv_and_fill_ -> recv_
simone-silvestri 8f672c5
Merge branch 'ss/fix-bc-slowdown' of github.com:CliMA/Oceananigans.jl…
simone-silvestri 8bb38f3
correct typo
simone-silvestri 06c8e6c
fix distributed tripolar tests
simone-silvestri df82140
hmmm, this works but not optimal
simone-silvestri 7132449
remove commented code
simone-silvestri 2870569
compact
simone-silvestri c0a2965
use same syntax
simone-silvestri e1e6c01
some cleanup
simone-silvestri 8c19e6d
this should make also the output writers work
simone-silvestri a02c7d5
split the two
simone-silvestri 801b494
fix kwcall issue
simone-silvestri 93918ba
add an inbounds
simone-silvestri 5fccb80
fix doctests
simone-silvestri 798d680
add this
simone-silvestri e182090
add double tests
simone-silvestri 437160f
scrape
simone-silvestri 5abe060
Update catke_vertical_diffusivity.jl
simone-silvestri 1ccc0ae
Update distributed_zipper.jl
simone-silvestri f54c010
Update halo_communication.jl
simone-silvestri 3a4c018
bugfix
simone-silvestri 0301b59
Merge branch 'ss/fix-bc-slowdown' of github.com:CliMA/Oceananigans.jl…
simone-silvestri cb3f60e
Merge branch 'main' into ss/fix-bc-slowdown
simone-silvestri 4cd231c
fix tests
simone-silvestri fb45f35
Merge branch 'ss/fix-bc-slowdown' of github.com:CliMA/Oceananigans.jl…
simone-silvestri d4b71aa
update
simone-silvestri 8516087
import method
simone-silvestri 5e42431
launch!
simone-silvestri 6f0586e
adding the CuDeviceArray
simone-silvestri 4fe373a
Update field_boundary_conditions.jl
simone-silvestri 236289e
Apply suggestion from @navidcy
simone-silvestri 4bf3d21
Apply suggestion from @navidcy
simone-silvestri 41e0b2d
Apply suggestion from @navidcy
simone-silvestri be0b738
Merge branch 'main' into ss/fix-bc-slowdown
simone-silvestri 1184dc5
Merge branch 'main' into ss/fix-bc-slowdown
simone-silvestri d0359b6
give a name to the namedtuple
simone-silvestri d178c24
Implement adapt_structure for boundary conditions
simone-silvestri da61000
Fix adapt_structure to include ordered_bcs
simone-silvestri 7436103
test again
simone-silvestri 915d6e6
Update fill_halo_kernels.jl
simone-silvestri 4d39d90
Merge branch 'main' into ss/fix-bc-slowdown
simone-silvestri 89f8974
Merge branch 'main' into ss/fix-bc-slowdown
simone-silvestri a305538
Merge remote-tracking branch 'origin/main' into ss/fix-bc-slowdown
simone-silvestri File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,128 @@ | ||
| extract_bc(bcs, ::West) = tuple(bcs.west) | ||
| extract_bc(bcs, ::East) = tuple(bcs.east) | ||
| extract_bc(bcs, ::South) = tuple(bcs.south) | ||
| extract_bc(bcs, ::North) = tuple(bcs.north) | ||
| extract_bc(bcs, ::Bottom) = tuple(bcs.bottom) | ||
| extract_bc(bcs, ::Top) = tuple(bcs.top) | ||
|
|
||
| extract_bc(bcs, ::BottomAndTop) = (bcs.bottom, bcs.top) | ||
| extract_bc(bcs, ::WestAndEast) = (bcs.west, bcs.east) | ||
| extract_bc(bcs, ::SouthAndNorth) = (bcs.south, bcs.north) | ||
|
|
||
| # In case of a DistributedCommunication paired with a | ||
| # Flux, Value or Gradient boundary condition, we split the direction in two single-sided | ||
| # fill_halo! events (see issue #3342) | ||
| # `permute_boundary_conditions` returns a 2-tuple containing the ordered operations to execute in | ||
| # position [1] and the associated boundary conditions in position [2] | ||
| function permute_boundary_conditions(bcs) | ||
|
|
||
| split_x_halo_filling = split_halo_filling(bcs.west, bcs.east) | ||
| split_y_halo_filling = split_halo_filling(bcs.south, bcs.north) | ||
|
|
||
| if split_x_halo_filling | ||
| if split_y_halo_filling | ||
| sides = [West(), East(), South(), North(), BottomAndTop()] | ||
| bcs_array = [bcs.west, bcs.east, bcs.south, bcs.north, bcs.bottom] | ||
| else | ||
| sides = [West(), East(), SouthAndNorth(), BottomAndTop()] | ||
| bcs_array = [bcs.west, bcs.east, bcs.south, bcs.bottom] | ||
| end | ||
| else | ||
| if split_y_halo_filling | ||
| sides = [WestAndEast(), South(), North(), BottomAndTop()] | ||
| bcs_array = [bcs.west, bcs.south, bcs.north, bcs.bottom] | ||
| else | ||
| sides = [WestAndEast(), SouthAndNorth(), BottomAndTop()] | ||
| bcs_array = [bcs.west, bcs.south, bcs.bottom] | ||
| end | ||
| end | ||
|
|
||
| perm = sortperm(bcs_array, lt=fill_first) | ||
| sides = tuple(sides[perm]...) | ||
|
|
||
| boundary_conditions = Tuple(extract_bc(bcs, side) for side in sides) | ||
|
|
||
| return sides, boundary_conditions | ||
| end | ||
|
|
||
| side_name(::West) = :west | ||
| side_name(::East) = :east | ||
| side_name(::South) = :south | ||
| side_name(::North) = :north | ||
| side_name(::Bottom) = :bottom | ||
| side_name(::Top) = :top | ||
| side_name(::WestAndEast) = :west_and_east | ||
| side_name(::SouthAndNorth) = :south_and_north | ||
| side_name(::BottomAndTop) = :bottom_and_top | ||
|
|
||
| # Split direction in two distinct fill_halo! events in case of a communication boundary condition | ||
| # (distributed DCBC), paired with a Flux, Value or Gradient boundary condition | ||
| split_halo_filling(bcs1, bcs2) = false | ||
| split_halo_filling(::DCBC, ::DCBC) = false | ||
| split_halo_filling(bcs1, ::DCBC) = true | ||
| split_halo_filling(::DCBC, bcs2) = true | ||
|
|
||
| # Same thing for MultiRegion boundary conditions | ||
| split_halo_filling(::MCBC, ::MCBC) = false | ||
| split_halo_filling(bcs1, ::MCBC) = true | ||
| split_halo_filling(::MCBC, bcs2) = true | ||
|
|
||
| # heterogenous distribute-shared communication is not supported | ||
| # TODO: support heterogeneous distributed-shared communication | ||
| split_halo_filling(::MCBC, ::DCBC) = throw("Cannot split MultiRegion and Distributed boundary conditions.") | ||
| split_halo_filling(::DCBC, ::MCBC) = throw("Cannot split MultiRegion and Distributed boundary conditions.") | ||
|
|
||
| ##### | ||
| ##### Halo filling order | ||
| ##### | ||
|
|
||
| const PBCT = Union{PBC, Tuple{Vararg{PBC}}} | ||
| const MCBCT = Union{MCBC, Tuple{Vararg{MCBC}}} | ||
| const DCBCT = Union{DCBC, Tuple{Vararg{DCBC}}} | ||
| const OBCTC = Union{OBC, Tuple{Vararg{OBC}}} | ||
|
|
||
| # Distributed halos have to be filled last to allow the | ||
| # possibility of asynchronous communication: | ||
| # If other halos are filled after we initiate the distributed communication, | ||
| # (but before communication is completed) the halos will be overwritten. | ||
| # For this reason we always want to perform local halo filling first and then | ||
| # initiate communication | ||
|
|
||
| # Periodic is handled after Flux, Value, Gradient because | ||
| # Periodic fills also corners while Flux, Value, Gradient do not | ||
| # TODO: remove this ordering requirement (see issue https://github.com/CliMA/Oceananigans.jl/issues/3342) | ||
|
|
||
| # Order of halo filling | ||
| # 1) Flux, Value, Gradient (TODO: remove these BC and apply them as fluxes) | ||
| # 2) Periodic (PBCT) | ||
| # 3) Shared Communication (MCBCT) | ||
| # 4) Distributed Communication (DCBCT) | ||
|
|
||
| # We define "greater than" `>` and "lower than", for boundary conditions | ||
| # following the rules outlined in `fill_first` | ||
| # i.e. if `bc1 > bc2` then `bc2` precedes `bc1` in filling order | ||
| @inline Base.isless(bc1::BoundaryCondition, bc2::BoundaryCondition) = fill_first(bc1, bc2) | ||
|
|
||
| # fallback for `Nothing` BC. | ||
| @inline Base.isless(::Nothing, ::Nothing) = true | ||
| @inline Base.isless(::BoundaryCondition, ::Nothing) = false | ||
| @inline Base.isless(::Nothing, ::BoundaryCondition) = true | ||
| @inline Base.isless(::BoundaryCondition, ::Missing) = false | ||
| @inline Base.isless(::Missing, ::BoundaryCondition) = true | ||
|
|
||
| fill_first(bc1::DCBCT, bc2) = false | ||
| fill_first(bc1::PBCT, bc2::DCBCT) = true | ||
| fill_first(bc1::DCBCT, bc2::PBCT) = false | ||
| fill_first(bc1::MCBCT, bc2::DCBCT) = true | ||
| fill_first(bc1::DCBCT, bc2::MCBCT) = false | ||
| fill_first(bc1, bc2::DCBCT) = true | ||
| fill_first(bc1::DCBCT, bc2::DCBCT) = true | ||
| fill_first(bc1::PBCT, bc2) = false | ||
| fill_first(bc1::MCBCT, bc2) = false | ||
| fill_first(bc1::PBCT, bc2::MCBCT) = true | ||
| fill_first(bc1::MCBCT, bc2::PBCT) = false | ||
| fill_first(bc1, bc2::PBCT) = true | ||
| fill_first(bc1, bc2::MCBCT) = true | ||
| fill_first(bc1::PBCT, bc2::PBCT) = true | ||
| fill_first(bc1::MCBCT, bc2::MCBCT) = true | ||
| fill_first(bc1, bc2) = true |
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
Oops, something went wrong.
Oops, something went wrong.
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.
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.
what does "ordered bcs" mean here? Shouldn't we store the ordering (eg something like a tuple that reads
(:west, :east, :top, :bottom, :south, :north)? If we store the kernels, can't the ordering be embedded in that object?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.
Maybe we can give it another name? Basically,
ordered_bcsholds the boundary conditions in the correct order to be used by the kernels. In practice, the redundant fields here are thewest,east,south,north, etc... the "meat" is inordered_bcs.