search

phorgue / porousMultiphaseFoam

A porous multiphase toolbox for OpenFOAM
Other
124 stars 64 forks source link

Fix `SrcExt` calculation in `impesFoam` and `anisoImpesFoam` #5

Closed FoamScience closed 5 years ago

FoamScience commented 5 years ago

The current implementation says:

SrcExt =  qExtraction/(Vinj+dimensionedScalar("",dimVol,SMALL));

Which works fine on test cases (injection/extraction) mainly because Vinj.value() == Vext.value() is true.

The line should spell out like:

SrcExt =  qExtraction/(Vext+dimensionedScalar("",dimVol,SMALL));

instead (Last line in createWellbores.H file)

I'm impressed no "Unused Variable" warning was issued when compiling as I can't see you disabled it anywhere, but a grep for "Vext" in "impesFoam" directory only displays the declaration, hence no using!!

I would appreciate any thoughts on this! Because I really like to think I'm "safe" against such bugs while working on OpenFOAM solvers!!

Also, I think It's better to immediately assign SrcExt value instead of init. to zero then assign (It's not that long :smile: ):

dimensionedScalar SrcExt("SrcExt",qExtraction/(sum(Wext*mesh.V())+dimensionedScalar("",dimVol,SMALL)));

As it may reduce the risk of having such bugs; Just my opinion though.

phorgue commented 5 years ago

Thanks :-) for report, bug has been corrected in the last commit.

FoamScience commented 5 years ago

:+1: For implementing it exactly as I suggested and for the fast response.

I expected GCC to throw a warning because the variable wasn't actually used but its initialization apparently had some side effects leading the compiler to think it was used. I work with GCC 5.4.0 20160609 (Ubuntu 5.4.0-6ubuntu1~16.04.10) which is the latest version in Xenial's stable repos.

I hear this was fixed in newer versions of GCC but I'm not brave enough to mess with my building system.