View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0002226 | OpenFOAM | Bug | public | 2016-09-01 14:47 | 2016-09-29 14:01 |
| Reporter | MattijsJ | Assigned To | henry | ||
| Priority | low | Severity | minor | Reproducibility | always |
| Status | resolved | Resolution | fixed | ||
| Platform | GNU/Linux | OS | OpenSuSE | OS Version | 13.2 |
| Product Version | dev | ||||
| Fixed in Version | dev | ||||
| Summary | 0002226: GAMGagglomeration does not stop when nCellsInCoarsestLevel 1 | ||||
| Description | set nCellsInCoarsestLevel to 1. The pairGAMGagglomeration keeps on iterating until it reaches the maxLevel (50). Hence it creates lots of unnecessary levels. Attached a dump of the levels (with 'processorAgglomerator none') The easiest fix is probably to adapt the logic in GAMGAgglomeration::continueAgglomerating to include the old number of cells. | ||||
| Tags | No tags attached. | ||||
|
|
|
|
|
Can you provide a patch for this? It is unlikely that setting nCellsInCoarsestLevel to 1 is a good idea anyway. |
|
|
GAMGAgglomeration.patch (2,752 bytes)
diff --git a/src/OpenFOAM/matrices/lduMatrix/solvers/GAMG/GAMGAgglomerations/GAMGAgglomeration/GAMGAgglomeration.C b/src/OpenFOAM/matrices/lduMatrix/solvers/GAMG/GAMGAgglomerations/GAMGAgglomeration/GAMGAgglomeration.C
index 6f7665b..517aad3 100644
--- a/src/OpenFOAM/matrices/lduMatrix/solvers/GAMG/GAMGAgglomerations/GAMGAgglomeration/GAMGAgglomeration.C
+++ b/src/OpenFOAM/matrices/lduMatrix/solvers/GAMG/GAMGAgglomerations/GAMGAgglomeration/GAMGAgglomeration.C
@@ -206,11 +206,14 @@ void Foam::GAMGAgglomeration::compactLevels(const label nCreatedLevels)
bool Foam::GAMGAgglomeration::continueAgglomerating
(
+ const label nFineCells,
const label nCoarseCells
) const
{
// Check the need for further agglomeration on all processors
- bool contAgg = nCoarseCells >= nCellsInCoarsestLevel_;
+ bool contAgg =
+ (nCoarseCells >= nCellsInCoarsestLevel_)
+ && (nCoarseCells < nFineCells);
mesh().reduce(contAgg, andOp<bool>());
return contAgg;
}
diff --git a/src/OpenFOAM/matrices/lduMatrix/solvers/GAMG/GAMGAgglomerations/GAMGAgglomeration/GAMGAgglomeration.H b/src/OpenFOAM/matrices/lduMatrix/solvers/GAMG/GAMGAgglomerations/GAMGAgglomeration/GAMGAgglomeration.H
index fd94b55..7019c67 100644
--- a/src/OpenFOAM/matrices/lduMatrix/solvers/GAMG/GAMGAgglomerations/GAMGAgglomeration/GAMGAgglomeration.H
+++ b/src/OpenFOAM/matrices/lduMatrix/solvers/GAMG/GAMGAgglomerations/GAMGAgglomeration/GAMGAgglomeration.H
@@ -152,7 +152,11 @@ protected:
void compactLevels(const label nCreatedLevels);
//- Check the need for further agglomeration
- bool continueAgglomerating(const label nCoarseCells) const;
+ bool continueAgglomerating
+ (
+ const label nCells,
+ const label nCoarseCells
+ ) const;
//- Gather value from all procIDs onto procIDs[0]
template<class Type>
diff --git a/src/OpenFOAM/matrices/lduMatrix/solvers/GAMG/GAMGAgglomerations/pairGAMGAgglomeration/pairGAMGAgglomerate.C b/src/OpenFOAM/matrices/lduMatrix/solvers/GAMG/GAMGAgglomerations/pairGAMGAgglomeration/pairGAMGAgglomerate.C
index bf4c695..42133e6 100644
--- a/src/OpenFOAM/matrices/lduMatrix/solvers/GAMG/GAMGAgglomerations/pairGAMGAgglomeration/pairGAMGAgglomerate.C
+++ b/src/OpenFOAM/matrices/lduMatrix/solvers/GAMG/GAMGAgglomerations/pairGAMGAgglomeration/pairGAMGAgglomerate.C
@@ -54,7 +54,7 @@ void Foam::pairGAMGAgglomeration::agglomerate
*faceWeightsPtr
);
- if (continueAgglomerating(nCoarseCells))
+ if (continueAgglomerating(finalAgglomPtr().size(), nCoarseCells))
{
nCells_[nCreatedLevels] = nCoarseCells;
restrictAddressing_.set(nCreatedLevels, finalAgglomPtr);
|
|
|
On very large clusters (6k procs) we've seen a benefit - smaller nCellsInCoarsestLevel the better. I've attached a patch but not happy with the logic inside continueAgglomerating. - with the old logic we stop if one processor < nCellsInCoarsestLevel - do we want all processors instead? - the additional logic should be to continue if at least one processor has nCoarseCells<nFineCells. - an alternative is to work on averages: label nTotalCoarseCells = returnReduce(nCoarseCells, sumOp<label>()); if (nTotalCoarseCells >= Pstream::nProcs()*nCellsInCoarsestLevel_) { return true; } label nTotalFineCells = returnReduce(nFineCells, sumOp<label>()); return ((nTotalCoarseCells < nTotalFineCells); |
|
|
Which option provide best performance on a range of cases, decompositions and number of processors? |
|
|
Not sure. 1. for the nCellsInCoarsestLevel_ there are three options: 1A as is now, if one processor is below nCellsInCoarsestLevel_ stop 1B if all processors are below nCellsInCoarsestLevel_ stop 1C if average is below nCellsInCoarsestLevel_ stop for 'normal' decompositions they all behave the same but if we weight the decomposition (e.g. by number of lagrangian particles) and one processor has 100 cells and all others 1M it is the 100 cells processor that stops all agglomeration (with 1A). 2. Automatic stop (nCoarseCells=nFineCells): this should be over all processors so only stop if none of the processors has changed. So either 2A: returnReduce(nCoarseCells < nFineCells, orOp<bool>()) or 2B: returnReduce(nCoarseCells, sumOp<label>()) < returnReduce(nFineCells, sumOp<label>()) 2B will also work once we go to agglomeration methods where cells can migrate across processors. My vote is for the 1B + 2B. |
|
|
I agree with this proposal. Which option is implemented in the patch you supplied? |
|
|
Not that one. Just apply the patch and replace the logic in continueAgglomerating with label nTotalCoarseCells = returnReduce(nCoarseCells, sumOp<label>()); if (nTotalCoarseCells >= Pstream::nProcs()*nCellsInCoarsestLevel_) { return true; } label nTotalFineCells = returnReduce(nFineCells, sumOp<label>()); return nTotalCoarseCells < nTotalFineCells; You could cache the nTotalCoarseCells in the calling function and use it to avoid the reduction for nTotalFineCells but I don't think that is worth the effort. |
|
|
Resolved by commit faa6d0e4dad9530a05fc4015d3af213950e7df52 |
|
|
Logic is slightly wrong and should only test for no-change if nCoarseCells is still larger than wanted nCellsInCoarsestLevel. Below works: const label nTotalCoarseCells = returnReduce(nCoarseCells, sumOp<label>()); if (nTotalCoarseCells < Pstream::nProcs()*nCellsInCoarsestLevel_) { return false; } else { const label nTotalFineCells = returnReduce(nFineCells, sumOp<label>()); return nTotalCoarseCells < nTotalFineCells; } |
|
|
Can be closed. Solved by b4bf4be70081219fe654ba092e766b718beabd7c. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2016-09-01 14:47 | MattijsJ | New Issue | |
| 2016-09-01 14:47 | MattijsJ | File Added: log.potentialFoam_nCells1 | |
| 2016-09-03 16:06 | henry | Note Added: 0006805 | |
| 2016-09-03 20:50 | MattijsJ | File Added: GAMGAgglomeration.patch | |
| 2016-09-03 20:57 | MattijsJ | Note Added: 0006811 | |
| 2016-09-04 21:10 | henry | Note Added: 0006821 | |
| 2016-09-06 12:21 | MattijsJ | Note Added: 0006831 | |
| 2016-09-06 12:33 | henry | Note Added: 0006832 | |
| 2016-09-06 12:51 | MattijsJ | Note Added: 0006834 | |
| 2016-09-06 13:31 | henry | Assigned To | => henry |
| 2016-09-06 13:31 | henry | Status | new => resolved |
| 2016-09-06 13:31 | henry | Resolution | open => fixed |
| 2016-09-06 13:31 | henry | Fixed in Version | => dev |
| 2016-09-06 13:31 | henry | Note Added: 0006836 | |
| 2016-09-16 10:11 | MattijsJ | Status | resolved => feedback |
| 2016-09-16 10:11 | MattijsJ | Resolution | fixed => reopened |
| 2016-09-16 10:11 | MattijsJ | Note Added: 0006869 | |
| 2016-09-29 13:59 | MattijsJ | Note Added: 0006942 | |
| 2016-09-29 13:59 | MattijsJ | Status | feedback => assigned |
| 2016-09-29 14:01 | henry | Status | assigned => resolved |
| 2016-09-29 14:01 | henry | Resolution | reopened => fixed |
