View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0003074 | OpenFOAM | Feature | public | 2018-09-21 16:58 | 2018-09-25 08:33 |
| Reporter | deOliveira-R | Assigned To | will | ||
| Priority | normal | Severity | feature | Reproducibility | always |
| Status | closed | Resolution | suspended | ||
| Platform | GNU/Linux | OS | Ubuntu | OS Version | 18.04 |
| Summary | 0003074: Improve pisoControl behaviour | ||||
| Description | I'm dealing a lot with the solution/convergence/correctorConvergence and derived classes at the moment, implementing a self contained fluid dynamics solver class, which is not the typical OpenFOAM style, but my problem is complex enough to demand it. While doing so, I encountered a few things that might be worth taking a look into. For chtMultiRegionFoam style solvers, currently in the fvSolution dictionaries there are the following controls (just an example): PIMPLE { residualControl { U 1e-5; } outerCorrectorResidualControl { U { tolerance 1e-6; relTol 0; } } } where: - residualControl ends the simulation if the control for all regions are satisfied (inherited by pimpleNoLoopControl from singleRegionSolutionControl) - outerCorrectorResidualControl ends the PIMPLE loop and advances the time step if the control for all regions are satisfied (inherited by pimpleNoLoopControl from singleRegionCorrectorConvergenceControl by defining "outerCorrector" as loop name). I noticed though that there seems to be no "innerCorrectorResidualControl" because the pisoControl class (which is inherited by pimpleNoLoopControl) does not inherit from singleRegionCorrectorConvergenceControl defining a "innerCorrector" loop. This implementation would allow the PISO loop to reduce the number of iterations based on some p/p_rgh tolerance, right? It is probably more complicated than just that, but I think I passed the intention. This seems like a very desirable feature. I'm usually not so cautious about using 50 or whatever outer correctors by defining outerCorrectorResidualControl but I am very careful about increasing the number of PISO solutions since I cannot control that. On that note, I would find it very good if the correct() method implemented by pisoControl and inherited by pimpleNoLoopControl would be renamed to correctPiso() or something, in line with correctNonOrthogonal(), which is inherited from nonOrthogonalSolutionControl. Firstly because it makes explicit what the method is supposed to do but also because correct() is usually the method by which some model solves/updates *all* its equations/values (turbulence, thermophysical model, etc). Also, since I'm already here, bitching about a bunch of great functionality that I use everyday (sorry about it!), I might as well mention that there is a typo on line 148 of singleRegionCorrectorConvergenceControl.C, causing it to print "Calclations" instead of "Calculations" (missing u there). | ||||
| Tags | No tags attached. | ||||
|
|
I am mostly in agreement. Inner-corrector/PISO convergence control has never been implemented (to my knowledge). I haven't tried to fit the bits together to make it work, but it should be possible; I tried to design it generally enough. I suspect the inheritance pattern might need to change, as having the same class inherit from two corrector convergence controls would't work. I would guess that we would make them compose the convergence control instead. How generally useful inner-corrector control is, I don't know. You would clearly value it. I do not know if you are alone. The naming conventions are a bit silly, I agree. The implementation as it stands did not change them from how they were previously. The problem is that the names for the iteration counters were not (and still are not) the same as the corresponding convergence controls. I'd love to make it all consistent, but you would potentially break every existing case, so it's not a decision to be taken lightly. We could change the name of the correct() method just as happily. Again, anyone with a custom solver is going to find that it doesn't build any more, but that's maybe less of a problem than breaking cases. Also I would probably change the loop(), loop(runTime&) overload, if I had free reign. I'd want to rename loop(), too. Maybe correctOuter() and correctInner()? |
|
|
This sort of work could be achieved by allocating maintenance funding (see https://openfoam.org/maintenance/), but we need to ask those funding the project whether they would value this, and/or if they care about the compatibility issues associated with changing the naming convention. We will do this by means of the Hub (https://hub.openfoam.org). This is a messaging platform which will be set up in the near future where we can have these conversations with users who have purchased maintenance plans. I am closing this report because this isn't the place for feature requests any more. I will raise the issue on the Hub once it is up and running (or you can, if you have a maintenance plan). |
|
|
Also, I fixed the typo. Thanks for pointing that out. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2018-09-21 16:58 | deOliveira-R | New Issue | |
| 2018-09-24 13:38 | will | Note Added: 0010071 | |
| 2018-09-24 13:39 | will | Note Added: 0010072 | |
| 2018-09-24 13:39 | will | Note Added: 0010073 | |
| 2018-09-24 13:40 | will | Assigned To | => will |
| 2018-09-24 13:40 | will | Status | new => closed |
| 2018-09-24 13:40 | will | Resolution | open => suspended |
| 2018-09-25 08:33 | will | Note Edited: 0010072 |
