View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0001828OpenFOAMBugpublic2015-08-15 22:482015-09-02 14:40
Reporterwyldckat Assigned Tohenry  
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Summary0001828: function object library "fvTools" has issues with cached fields
DescriptionFor example, if we take the tutorial "incompressible/simpleFoam/pipeCyclic", which has in "system/fvSolution" this block:

    cache
    {
        grad(U);
    }

And now we add to "system/controlDict" file this block:

    functions
    {
      MycalcFvcGrad
      {
          type calcFvcGrad;
          functionObjectLibs ("libFVFunctionObjects.so");
          fieldName "U";
          resultName "Ug";
      }
    }


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 InformationThe post on the forum that lead me to this: http://www.cfd-online.com/Forums/openfoam-post-processing/117099-calcfvcgrad.html#post558372
TagsNo tags attached.

Activities

wyldckat

2015-08-15 22:48

updater  

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)   

henry

2015-08-18 11:55

manager   ~0005259

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

    resultName "none";

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.

wyldckat

2015-08-18 18:30

updater   ~0005263

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.

henry

2015-08-18 18:42

manager   ~0005264

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.

henry

2015-08-18 19:32

manager   ~0005266

Immediate problem is resolved but more work needs to be done to improve the operation of the fvTools.

Issue History

Date Modified Username Field Change
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