fixedNormalSlipFvPatchField does not write its value

assigned to @kuti

Thanks a lot for your contribution @johan_roenby, highly appreciated.

I will submit a commit on behalf of you.

Though, I think we need to migrate to another system which can allow non-developer merge requests and forks. What do you think?

mentioned in commit a9fd39c6

mentioned in merge request !421 (closed)

It does seem sensible to have a permission level for external contributors who can fix the bugs we encounter in a merge request. Would probably make life easier for the developers and approach the spirit of a collaborative open source project. I am, however, not an expert in git workflow strategies and the potential implications of allowing non-developer merge requests. If you decide not to allow merge requests from external contributors, it may be a good idea to mention in the section "Submitting for code review" here that only Developers are allowed to submit merge requests.

Hi @johan_roenby - I'm not really sure that any solution is particularly optimal. We don't have or need "value" for a slip condition. Adding it to fixedSlipNormal seems to be only related to visualizing it in paraview and no other purpose. Ideally would like paraview to do something useful with the fixedValue entry, but this has other problems. Would need to apply the patch-normal transformation within paraview, but at the point of handling the BC, the geometry information is marginally accessible and it would also start to look like re-implementing a large variety of BCs within VTK. Also, at the moment the fixedValue is simply a field, but could arguable be a PatchFunction1 to allow temporal-spatial variations. Parsing and handling these in paraview is a complete no-go.

Second thought: if we agree to disagree, why not add support for "writeValue" to slip conditions? Eg,

patch1
{
    type  fixedNormalSlip;
    writeValue  true;
    fixedValue  ...;
}

This would only add an extra byte to the class size, and make the output configurable. @andy @Mattijs ?

Implementation of write would look something like this:

if (writeValue_)
{
    os.writeEntry("writeValue", "true");
    this->writeEntry("value", os);
}

Make it a global setting in etc/controlDict so users running with non-paraFoam postprocessing can set up their environment? All of zeroGradient, fixedNormalSlip etc should look at this and output value field. Maybe even put it in fvPatchField itself but then you might have two value entries... not a good idea. But there are not that many bcs that do not output the value anyway.

I think the writeValue option is a good idea. I would set it to true as default. The amount of extra (in principle redundant) boundary data written to the time directories will always be a small fraction of the internal fields written out. And it is vital that paraview shows users what actually is going on on the boundary.

mentioned in commit 87717be3

unassigned @kuti

mentioned in commit f816663f

Hi @johan_roenby,

Could you please tell me if there is any other BC for which the writeValue solution could be applicable?

Hi!

For example noSlip? Paraview process it in a weird way like it is a slip (like) BC. It show you a really misleading result. This is the only reason I don't use it while I saw that many people use this BC and postprocess in paraview which I think is not correct at all. And for this BC the value is one line (uniform value) so the extra data is not a reason for not writing it. And this is just one widely used BC which gives you wrong results in paraview...

For example in the cavity tutorial the left is the default, right has the extra "value uniform (0 0 0);" entry:

Boundary values are such an essential part of a CFD solution that I think every boundary condition should at least have the option to write out its value to allow the user to discover problems associated with wrong BC setup with paraview (or direction inspection of data files). Going to $FOAM_SRC/finiteVolume/fields/fvPatchFields/derived and doing

grep -r -L "writeEntry(\"value\", os)" . --include=*Field.C

I get the following list of BC's not writing out their value directly (they may do it by calling another function doing it):

./mappedFixedInternalValue/mappedFixedInternalValueFvPatchField.C
./fixedNormalSlip/fixedNormalSlipFvPatchField.C
./uniformJump/uniformJumpFvPatchField.C
./codedMixed/codedMixedFvPatchField.C
./fixedFluxExtrapolatedPressure/fixedFluxExtrapolatedPressureFvPatchScalarField.C
./uniformJumpAMI/uniformJumpAMIFvPatchField.C
./scaledFixedValue/scaledFixedValueFvPatchField.C
./surfaceNormalFixedValue/surfaceNormalFixedValueFvPatchVectorField.C
./turbulentDigitalFilterInlet/turbulentDigitalFilterInletFvPatchVectorField.C
./mappedFixedPushedInternalValue/mappedFixedPushedInternalValueFvPatchField.C
./fanPressure/fanPressureFvPatchScalarField.C
./partialSlip/partialSlipFvPatchField.C
./mappedMixed/mappedMixedFvPatchField.C
./mappedField/mappedMixedFieldFvPatchField/mappedMixedFieldFvPatchField.C
./noSlip/noSlipFvPatchVectorField.C
./slip/slipFvPatchField.C
./swirlFanVelocity/swirlFanVelocityFvPatchField.C
./fixedInternalValueFvPatchField/fixedInternalValueFvPatchField.C
./rotatingTotalPressure/rotatingTotalPressureFvPatchScalarField.C
./fan/fanFvPatchField.C
./pressureInletUniformVelocity/pressureInletUniformVelocityFvPatchVectorField.C

Ideally these should be checked to see whether their write function writes their value. If this is too much we can limit ourselves to the BC's that involve slip, e.g. found using:

grep -r "transform(I" . --include=*Field.C

This gives:

./fixedNormalSlip/fixedNormalSlipFvPatchField.C:        (nHat*(nHat & fixedValue_) + transform(I - sqr(nHat), pif)) - pif
./fixedNormalSlip/fixedNormalSlipFvPatchField.C:      + transform(I - sqr(nHat), this->patchInternalField())
./pressureInletOutletVelocity/pressureInletOutletVelocityFvPatchVectorField.C:    tmp<vectorField> transformGradValue = transform(I - valueFraction(), pvf);
./partialSlip/partialSlipFvPatchField.C:      + (1.0 - valueFraction_)*transform(I - sqr(nHat), pif) - pif
./partialSlip/partialSlipFvPatchField.C:       *transform(I - sqr(nHat), this->patchInternalField())
./fixedNormalInletOutletVelocity/fixedNormalInletOutletVelocityFvPatchVectorField.C:    tmp<vectorField> transformGradValue = transform(I - valueFraction(), pvf);

Of these pressureInletOutletVelocity and fixedNormalInletOutletVelocity already have writeEntry("value", os); in their write function but as far as I can see the partialSlip does not write out its value, so this could be given the writeValue option.

This on, on the other hand, could easily be added to paraview.

Added a VTK isssue 18085

mentioned in commit 100ae5c2

Hi @johan_roenby, is there any chance for you to test the recent change? thanks.

Hi @kuti

I now compiled the latest develop branch and tested my floating2Dbox case (attached to issue #1979 ).

In this case I chose deltaT = endTime = 0.1 to get CFL < 1.

I then set the floatingObject U boundary condition to: ´´´ type fixedNormalSlip; fixedValue uniform (1 0 0); writeValue true; ´´´ This makes the box in the middle of the domain suck in fluid at the back and blow out water in the front with slip on the sides.

To make it run without crashing, I had to make the change to adjustPhi.H suggested in #1979 .

I confirmed by inspecting the 0.01/U file that indeed the boundary values are now written to file for the floatingObject boundary.

I also loaded the velocity field in paraview. A comparison of the velocity field with writeValue = true (grey vectors) and writeValue = false (red vectors) is shown in this figure:

In the top and bottom of the floatingObject patch it wrongly appears as if the velocity field has non-zero normal components because paraview simply copies the closest cell values to the floatingObject patch.

If we were to restart the simulation using the writeValue = false files wouldn't we miss the information about the tangential velocity components? If so, I would argue that writeValue should always be true and hence the switch is not needed.

Hi @johan_roenby

It is still a matter of interpretation or perspective. From the OpenFOAM perspective, writing out the value field is useless, since it is not required for restart and could potentially contain anything arbitrary, and it unnecessarily bloats the file sizes. From a post-processing perspective, you are 100% correct that visualizing this BC in ParaView/VTK is very misleading. The compromise solution may be to continue with "writeValue false" as a default, but provide a general static-level switch to toggle these on.

Would still have to scour the code to find all of the BCs that could benefit from this.

Hi @mark Okay, I get the point: My argument was that the tangential component of the field on the patch is lost if it is not written to file. But since the patch works by simply copying this tangential component from the closest cell, the tangential component data can still be reconstructed at restart even if it is not written to file. And writting it to file introduces ambiguity about whether to read the tangential data from the file or reconstructing it from the cell values.

fixedNormalSlipFvPatchField does not write its value

Child items

Activity

Admin message

fixedNormalSlipFvPatchField does not write its value

Child items

Linked items

Related merge requests

Activity