View Issue Details

IDProjectCategoryView StatusLast Update
0003470OpenFOAMFeaturepublic2020-03-21 18:55
ReporterFederico MunicchiAssigned Tohenry 
PrioritynoneSeverityfeatureReproducibilityalways
Status closedResolutionno change required 
PlatformLinuxOSUbuntuOS Version18.04
Product Version7 
Fixed in Version 
Summary0003470: Inconsistent equation relaxation
DescriptionThe implications of line
https://github.com/OpenFOAM/OpenFOAM-7/blob/master/src/finiteVolume/fvMatrices/fvMatrix/fvMatrix.C#L630
are quite dramatic, to the point I have to remember to write every elliptic problem as
```
fvScalarMatrix ellipticM // Negative elliptic
(
    - fvm::laplacian(U)
   ==
    - Q
);
instead of

fvScalarMatrix ellipticM //Positive elliptic
(
     fvm::laplacian(U)
   ==
     Q
);
if i need to use relaxation (and I often need to) and i want to avoid crashes.

As in the attached code (relaxationTest), when one applies relaxation the two matrices differ quite a lot, and not just by their sign.
Especially, the real part of the eigenvalues becomes always positive!

In fact, relaxation works fine for the first matrix because the eigenvalues have a positive real part already, thus relaxation does not alter
the nature of the problem.
In the second matrix instead, the eigenvalues have negative real part, and thus relaxation changes the matrix from negative defined to positive defined.

For your convenience, I attached a working example where this issue leads to a crash (is it the reason why the pressure equation is never relaxed?).
I attached a version of icoFoam (icoPRelaxFoam) with a relaxation of the pressure equation and a switchable sign of the elliptic problem (that can be controlled from
transportProperties) and a cavity test.
You will see that when the relaxation is activated and the sign is positive, the solver crashes, while with negative sign the solver works just fine with the relaxed matrix.

I implemented a simple fix in the attached relaxationTest.C, which consists in ensuring that the diagonal keeps the right sign. As you see, this can be trivially implemented in fvMatrix.

I hope this was useful.
 
Steps To ReproduceEverything is packed in icoPRelaxFoam.zip
Simply compile the solver and run the included cavity example. Switch the negativeElliptic keyword in transportProperties from true to false to reproduce the issue.

You can also use the solver in relaxationTest.zip, which simply prints out small matrices before and after relaxation.
The fix to this bug is implemented there.
You can use the test folder (blockMesh followed by relaxationTest... I forgot the Allrun sorry) to check the effect on the matrix.
The fix ensures that the relaxed matrices (positive and negative elliptic) are the same but with opposite sign, as it should be.
TagsNo tags attached.

Activities

Federico Municchi

2020-03-21 14:59

reporter  

icoPRelaxFoam.zip (9,051 bytes)
relaxationTest.zip (6,283 bytes)

henry

2020-03-21 15:52

manager   ~0011266

It is essential to formulate the equation so that the diagonal is positive when applying relaxation. Testing the sign of the diagonal is too risky because for some equations the diagonal might be mixed positive and negative before relaxation and we have to assume what it should be after relaxation and we choose that it should be positive.

Federico Municchi

2020-03-21 16:22

reporter   ~0011267

Thank you for the kind comment.

Why not just scaling the terms in the diagonal keeping the same (mixed) signature to ensure relax(-A) = - relax(A)?
In this case, the mixed nature would be preserved in the eigenvalues.
Isn' t imposing that all the eigenvalues of A are positive a bit too dramatic since it leads to the aforementioned issues in the OpenFOAM API?

henry

2020-03-21 16:38

manager   ~0011268

There are many other parts of OpenFOAM which currently rely on the diagonal of the equation matrix being positive, are you planning to have special treatment in all of those? I think we would need a sign switch in all of those parts so that the implementer of the equation declares which the sign of the diagonal is expected to be. This would add an enormous amount of clutter and maintenance overhead, wouldn't it be better to arrange the equations so that the diagonal is positive? Why is this a problem?

Federico Municchi

2020-03-21 18:45

reporter   ~0011269

Well, it is clearly a very technical and subtle pitfall for anyone who writes code using OpenFOAM. Especially since one would not expect the relax() function to perform such a dramatic operation on the matrix and it is not so obvious why a matrix should possess such a feature. Perhaps this could be added to section 4.6.2 of the user guide, so that programmers are aware of that.

henry

2020-03-21 18:53

manager   ~0011270

This would not need to be discussed in the user-guide which covers the usage of the solvers provided with OpenFOAM, it would need to be in a programmers guide.

henry

2020-03-21 18:55

manager   ~0011271

Changing OpenFOAM so that the diagonal of equation matrices can have either sign would be a significant development requiring funding and so far none of the sponsors of the project have requested it or even shown interest in it.

Issue History

Date Modified Username Field Change
2020-03-21 14:59 Federico Municchi New Issue
2020-03-21 14:59 Federico Municchi File Added: icoPRelaxFoam.zip
2020-03-21 14:59 Federico Municchi File Added: relaxationTest.zip
2020-03-21 15:52 henry Note Added: 0011266
2020-03-21 15:53 henry Priority immediate => none
2020-03-21 15:53 henry Severity crash => feature
2020-03-21 15:53 henry Category Bug => Feature
2020-03-21 15:53 henry Description Updated View Revisions
2020-03-21 15:53 henry Steps to Reproduce Updated View Revisions
2020-03-21 16:22 Federico Municchi Note Added: 0011267
2020-03-21 16:38 henry Note Added: 0011268
2020-03-21 18:45 Federico Municchi Note Added: 0011269
2020-03-21 18:53 henry Note Added: 0011270
2020-03-21 18:55 henry Assigned To => henry
2020-03-21 18:55 henry Status new => closed
2020-03-21 18:55 henry Resolution open => no change required
2020-03-21 18:55 henry Note Added: 0011271