View Issue Details
|ID||Project||Category||View Status||Date Submitted||Last Update|
|0001828||OpenFOAM||[All Projects] Bug||public||2015-08-15 22:48||2015-09-02 14:40|
|Fixed in Version|
|Summary||0001828: function object library "fvTools" has issues with cached fields|
|Description||For example, if we take the tutorial "incompressible/simpleFoam/pipeCyclic", which has in "system/fvSolution" this block:|
And now we add to "system/controlDict" file this block:
We run the tutorial and it crashes on the second time step, when it tries to calculate "grad(U)" for the turbulence fields, for the velocity equation.
I only spotted this due to a report on the forum, which lead me to pinpoint that the problem was in this file: src/postProcessing/functionObjects/fvTools/calcFvcGrad/calcFvcGradTemplates.C
After some debugging and testing, I finally discovered that the problem was that, e.g. this line:
field = fvc::grad(vf);
is apparently taking over the assignment of the object or somehow tampering with the cached result, I'm sure.
But if we switch to using the "operator==" it works just fine and it doesn't crash, e.g.:
field == fvc::grad(vf);
Attached is a patch for this class and for the other 2 classes on this library. I'm not familiar enough with OpenFOAM's inner workings to figure out if this is the most appropriate fix or not, but at least it seems to be the simplest one.
|Additional Information||The post on the forum that lead me to this: http://www.cfd-online.com/Forums/openfoam-post-processing/117099-calcfvcgrad.html#post558372|
|Tags||No tags attached.|
functionObjects_fvTools_assignment.patch (2,235 bytes)
diff --git a/src/postProcessing/functionObjects/fvTools/calcFvcDiv/calcFvcDivTemplates.C b/src/postProcessing/functionObjects/fvTools/calcFvcDiv/calcFvcDivTemplates.C index d756174..4bc7f69 100644 --- a/src/postProcessing/functionObjects/fvTools/calcFvcDiv/calcFvcDivTemplates.C +++ b/src/postProcessing/functionObjects/fvTools/calcFvcDiv/calcFvcDivTemplates.C @@ -44,7 +44,7 @@ void Foam::calcFvcDiv::calcDiv volScalarField& field = divField(resultName, vf.dimensions()); - field = fvc::div(vf); + field == fvc::div(vf); processed = true; } diff --git a/src/postProcessing/functionObjects/fvTools/calcFvcGrad/calcFvcGradTemplates.C b/src/postProcessing/functionObjects/fvTools/calcFvcGrad/calcFvcGradTemplates.C index c028628..de62dd6 100644 --- a/src/postProcessing/functionObjects/fvTools/calcFvcGrad/calcFvcGradTemplates.C +++ b/src/postProcessing/functionObjects/fvTools/calcFvcGrad/calcFvcGradTemplates.C @@ -98,7 +98,7 @@ void Foam::calcFvcGrad::calcGrad vfGradType& field = gradField<Type>(resultName, vf.dimensions()); - field = fvc::grad(vf); + field == fvc::grad(vf); processed = true; } @@ -108,7 +108,7 @@ void Foam::calcFvcGrad::calcGrad vfGradType& field = gradField<Type>(resultName, sf.dimensions()); - field = fvc::grad(sf); + field == fvc::grad(sf); processed = true; } diff --git a/src/postProcessing/functionObjects/fvTools/calcMag/calcMagTemplates.C b/src/postProcessing/functionObjects/fvTools/calcMag/calcMagTemplates.C index f9db936..d4dd4ac 100644 --- a/src/postProcessing/functionObjects/fvTools/calcMag/calcMagTemplates.C +++ b/src/postProcessing/functionObjects/fvTools/calcMag/calcMagTemplates.C @@ -87,7 +87,7 @@ void Foam::calcMag::calc volScalarField& field = magField<volScalarField>(resultName_, vf.dimensions()); - field = mag(vf); + field == mag(vf); processed = true; } @@ -98,7 +98,7 @@ void Foam::calcMag::calc surfaceScalarField& field = magField<surfaceScalarField>(resultName_, sf.dimensions()); - field = mag(sf); + field == mag(sf); processed = true; }
functionObjects_fvTools_assignment.patch (2,235 bytes)
A better solution is to avoid the transfer of the temporary and use
field = fvc::grad(vf)();
however there is still a potential issue if
in which case the grad field is caches twice so one of them cannot be in the database. This is a mess. Do these fields need to be cached at all? Could they be generated just for output? If they need to be cached for computation shouldn't the built-in caching mechanism be used rather than creating a completely new competing and incompatible one in functionObjects? Why is resultName mandatory?
This needs a rewrite but first we need to be clear of the purpose, is it just for output or are these fields used for other puproses? If so how long should they be cached for and who cleans-up?
In the meantime I will add the () to stop the crash.
Interesting, I had wrongly assumed the "div" and "mag" function objects needed the same fix... I tested the 3 (grad, div, mag) with the "cache" option in "fvSolution" and created a test solver just now with the latest 2.4.x and had no issues.
Oh... from what I could find, "cache" seems to be only supported for the "grad" and the "limiter" operations...
Regarding your questions: I believe that these 3 function objects (for "grad", "div" and "mag") have two main uses:
1- For post-processing outside of the solver;
2- For using the resulting fields for other calculations that are done with other function objects.
For both, the ability to name the resulting field comes in handy, so that the desired name is used, at least with other function objects. This is possibly why the name is mandatory, so that the user can trigger the error message of which keyword can be used.
The "none" option for the "resultName" seems to be a workaround to avoid object collision, e.g. between the cached "grad(U)" and the automatically named "fvc::grad(U)".
Caching the calculation itself is only useful for this function object only if a previous "grad" calculation can be reused (one from within the solver), therefore extra clean up shouldn't be necessary.
Caching the resulting field is because:
- it's not ensured that the calculation was previously cached or not, which is what will always happen with "div" and "mag".
- to avoid reallocating memory multiple times.
With the setting "none" the resulting field is named "grad(U)" and hence will clash with the cached field unless the class is rewritten to use the cache.
The calcFvcGrad class caches its own field, thus if the grad cache is used it is stored twice for the duration of the run. I think there needs to be more control in the functionObject to handle the lifetime of the fields they cache.
Basically I think most functionObjects need to be rewritten based on a common set of object control structures.
||Immediate problem is resolved but more work needs to be done to improve the operation of the fvTools.|
|2015-08-15 22:48||wyldckat||New Issue|
|2015-08-15 22:48||wyldckat||File Added: functionObjects_fvTools_assignment.patch|
|2015-08-18 11:55||henry||Note Added: 0005259|
|2015-08-18 18:30||wyldckat||Note Added: 0005263|
|2015-08-18 18:42||henry||Note Added: 0005264|
|2015-08-18 19:32||henry||Note Added: 0005266|
|2015-08-18 19:32||henry||Status||new => resolved|
|2015-08-18 19:32||henry||Resolution||open => fixed|
|2015-08-18 19:32||henry||Assigned To||=> henry|