patchkernel: initialize partitioning in the constructor #82
Conversation
andrea-iob
force-pushed
the
patchkernel.protect.communicator.changes
branch
from
62738e1
to
a5cb992
Compare
andrea-iob
force-pushed
the
patchkernel.protect.communicator.changes
branch
2 times, most recently
from
2b34dc7
to
fa0adff
Compare
andrea-iob
force-pushed
the
patchkernel.protect.communicator.changes
branch
from
fa0adff
to
2e324ba
Compare
andrea-iob
force-pushed
the
patchkernel.protect.communicator.changes
branch
2 times, most recently
from
340b7c2
to
3cc064e
Compare
andrea-iob
force-pushed
the
patchkernel.protect.communicator.changes
branch
from
3cc064e
to
8378252
Compare
|
After some back and forth, I think the best solution is to initialize partitioning in the constructor. I've updated the constructors of the patches that support partitioning to accept the extra arguments needed to initialize the partitioning. For now, it is still possible to initialize a serial patch and the partition the patch later, but this will not be possible in bitpit v1.8. @edoardolombardi @marcocisternino Do you see any problem in this approach? |
andrea-iob
changed the title
|
@edoardolombardi @marcocisternino I had to make some changes in PABLO to allow moving all the octants on the first process during in the constructor. Please check if the changes look fine. I think they may be still some issues when the partitioning moves all the octants on a rank that is not the first one. This is something that should not happen with the current code, but, if this is something we want to support, some additional work may be needed. |
andrea-iob
force-pushed
the
patchkernel.protect.communicator.changes
branch
2 times, most recently
from
82b4532
to
716c2af
Compare
andrea-iob
force-pushed
the
patchkernel.protect.communicator.changes
branch
from
716c2af
to
0e259e1
Compare
| @@ -1520,17 +1552,7 @@ void PatchKernel::partitioningCleanup() | |||
| */ | |||
| bool PatchKernel::isPartitioned() const | |||
There was a problem hiding this comment.
Should this wrapper function isPartitioned used at the place of isCommunicatorSet function? In this case is it better to use it everywhere even in PatchKernel and its derived classes to avoid the procreation of re-coding around, in case of change in the wrapper function?
There was a problem hiding this comment.
Done.
Please note that some functions that handle the communicator may be simplified when the deprecated partitioning functions (the ones that take the communicator as argument) are be removed.
andrea-iob
force-pushed
the
patchkernel.protect.communicator.changes
branch
from
0e259e1
to
a6d686a
Compare
|
There was a problem hiding this comment.
@andrea-iob just check my previous comments before merging, please.
andrea-iob
force-pushed
the
patchkernel.protect.communicator.changes
branch
from
a6d686a
to
944401d
Compare
The same limitation exists in the current master: the restore function has to be called by the same number of processes that have called the dump function, this means that for restoring a patch it is necessary to know how it was dumped. To overcome this limitation, dump and restore should be handled by the PatchManager. Before dumping the patch, the manager could write some additional information that can later be used to identify how the patch should be restored. This will also allow to restore a patch without knowing the type of the patch (e.g., VolOctree, SurFunstructured, ...). |
andrea-iob
force-pushed
the
patchkernel.protect.communicator.changes
branch
from
944401d
to
79b0f3f
Compare
|
I fixed the evaluation of PABLO maximum depth when the tree is empty. Also, it seems that the maximum depth was not updated after coarsening. @marcocisternino @edoardolombardi Could you please check if the changes are fine ("PABLO: fix evaluation of maximum depth when the tree is emtpy" 84f4096 and "PABLO: update maximum depth after coarsening the tree" fb5e42a)? |
…on of the tree is null
If the patch is not partitioned there is no need to set the communicator.
Indexes are declared as std::size_t, they cannot be less than zero.
If a valid communicator is passed to the constructor, a partitioned patch will be created, otherwise the created patch will be serial. Patches that are filled automatically (e.g. VolOctree) will initialize the cells only on the process identified by the rank zero in the communicator.
andrea-iob
force-pushed
the
patchkernel.protect.communicator.changes
branch
from
79b0f3f
to
6620ad4
Compare
|
Max local depth fixing seems good to me. I tested the branch with a complex mesh using CFD solver. No problem found. |
Setting the communicator can now be done only when the patch is partitioned.