-
Notifications
You must be signed in to change notification settings - Fork 15
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
Fix CommonSpaces and MPI #2176
Fix CommonSpaces and MPI #2176
Conversation
19f1a7f
to
a7ec52d
Compare
a7ec52d
to
10dcf4d
Compare
10dcf4d
to
59bc0f4
Compare
# Column spaces are not supported with MPI | ||
if !(ClimaComms.context() isa ClimaComms.MPICommsContext) |
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.
Does this just not work?
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 had this error:
AssertionError: Columns can only be created on Singleton contextes.
But maybe it's because I am not caught up to main. Let me try rebasing
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.
Actually, I added that @assert
. I'll verify again
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 added the assert because creating columns in MPI settings is not possible:
ClimaCore.jl/src/Topologies/interval.jl
Line 46 in c19162a
@assert context isa ClimaComms.SingletonCommsContext |
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.
Here's the failing build: https://buildkite.com/clima/climacore-ci/builds/5255
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.
Overall this PR looks like a good set of fixes / improvements.
0c105bc
to
76896fd
Compare
The `CommonGrids` module passes down the context to all the grid it creates. This is a problem for IntervalTopology, because it can only be created with Singletons. This commit fixes this issue and adds test checking that we can create CommonSpaces with MPI and CUDA
76896fd
to
d9957d6
Compare
The
CommonGrids
module passes down the context to all the grid it creates. This is a problem for IntervalTopology, because it can only be created with Singletons.This commit fixes this issue and adds test checking that we can create CommonSpaces with MPI and CUDA
Closes #2175