Multiple independent volumes #280
Conversation
majosm
force-pushed
the
multiple-independent-volumes
branch
3 times, most recently
from
f328d36 to
51ea9ea
Compare
|
Back to draft for a bit (just noticed a spot in the MPI communication code where it's assuming single volume). |
majosm
force-pushed
the
multiple-independent-volumes
branch
from
51ea9ea to
12938f1
Compare
|
@inducer Fixed it (and added multi-volume MPI-enabled example in illinois-ceesd/mirgecom#726 to test it.) |
|
Also threw the |
majosm
force-pushed
the
multiple-independent-volumes
branch
from
91deaa7 to
4cad825
Compare
inducer
force-pushed
the
multiple-independent-volumes
branch
from
4cad825 to
42fa3ad
Compare
| e = dcoll.distributed_boundary_swap_connection(trace_dd)(i) | ||
| assert isinstance(trace_dd.domain_tag, BoundaryDomainTag) | ||
| if trace_dd.domain_tag.tag is FACE_RESTR_INTERIOR: | ||
| e = dcoll.opposite_face_connection(trace_dd.domain_tag)(i) |
There was a problem hiding this comment.
This seems to have lost the ability to do distributed. How come?
There was a problem hiding this comment.
This was originally removed in #236, I guess in part because distributed_boundary_swap_connection was also removed. I didn't remove that method in this PR, so I restored the call here. I'm a little bit confused about what's actually going on in this code though... I thought the boundary swap connection was remote-to-local, not local-to-remote as it appears to be used here?
There was a problem hiding this comment.
Why do you think this is used as local-to-remote here? Not sure I follow this. At any rate, I've backed this out, we'll refit distributed support to this if/when needed.
There was a problem hiding this comment.
Sorry, I shouldn't have said local-to-remote -- that wouldn't make sense either. What I meant is that it's applying a connection that goes remote-to-local to something that's already on the local boundary discr (i). That seems at best redundant (if the discrs are the same) and at worst wrong (if they're different), right?
inducer
force-pushed
the
multiple-independent-volumes
branch
from
7e0380b to
0dc755e
Compare
inducer
left a comment
inducer
left a comment
There was a problem hiding this comment.
LGTM, thanks! And sorry about the epic delay in getting this in.
| e = dcoll.distributed_boundary_swap_connection(trace_dd)(i) | ||
| assert isinstance(trace_dd.domain_tag, BoundaryDomainTag) | ||
| if trace_dd.domain_tag.tag is FACE_RESTR_INTERIOR: | ||
| e = dcoll.opposite_face_connection(trace_dd.domain_tag)(i) |
There was a problem hiding this comment.
Why do you think this is used as local-to-remote here? Not sure I follow this. At any rate, I've backed this out, we'll refit distributed support to this if/when needed.
2 participants
Adds the subset of changes from #236, #239, #243, and #246 needed to support multiple non-interacting volume discretizations in a discretization collection. Notably, this PR includes all of the interface changes needed to make illinois-ceesd/mirgecom#726 mergeable.