View Issue Details

IDProjectCategoryView StatusLast Update
0002216OpenFOAMPatchpublic2016-08-24 19:36
Reporterwyldckat Assigned Tohenry  
PrioritylowSeveritytweakReproducibilityN/A
Status resolvedResolutionfixed 
Fixed in Versiondev 
Summary0002216: A few code revisions for the "fieldValues" and "forces" function objects
DescriptionWhile I was looking into issue #2214, I found a few more issues that could/should be sorted out. The attached package "tweaks_for_a_few_function_objects_v1.tar.gz" (for OpenFOAM 4.x) provides two categories of tweaks:

  A. Renamed local for loop variables "i" to contextual names (e.g. "fieldi"), although only when being used inside the method "writeFileHeader(const label i)". I didn't continue updating the others that use "i" as the iterator counter on those classes, since there is no explicit rule for it in the coding style guide.

  B. Further revising of the forces library, at least as an attempt to minimize future issues with file export (we've have 3 or 4 bugs with this specific issue).


Files provided for category A:

  src/functionObjects/field/fieldValues/fieldValueDelta/fieldValueDelta.C
  src/functionObjects/field/fieldValues/surfaceRegion/surfaceRegion.C
  src/functionObjects/field/fieldValues/volRegion/volRegion.C

Files provided for category B:

  src/functionObjects/forces/forceCoeffs/forceCoeffs.C
  src/functionObjects/forces/forces/forces.C
  src/functionObjects/forces/forces/forces.H


The modifications made to the forces library were as follows:

  - New enumeration "fileID", to help enforce which number refers to which file name.

  - Bug fix: in "forceCoeffs::writeFileHeader" was outputting twice the "Cm Cd Cl ..." entry.

  - Some header text was repeated and has been deferred to a "const word" for ensuring that "need only to be fixed once".

  - In both classes, the enumeration is now enforced for the most part (only exception is in "writeFileHeader", after confirming that the ID is valid), whenever it's necessary to write to a specific file.

    - This isn't bullet-proof, but the alternative requires using named-index lists (which was likely the original idea behind using hash-lists a few years ago).


Additional InformationI'll provide the updated patch for OpenFOAM-dev in a few minutes... I had forgotten about the several changes already done there.

Nonetheless, I'm still uploading the patch for 4.x, since it was the one I tested things already. Although it might not make much sense to apply it to 4.x... but the bug fix for "forceCoeffs::writeFileHeader" is still useful.
TagsNo tags attached.

Activities

wyldckat

2016-08-24 15:46

updater  

henry

2016-08-24 15:50

manager   ~0006743

We should apply bug-fixes to OpenFOAM-4.x and develop OpenFOAM-dev. Could you let me know which change is the 'bug fix for "forceCoeffs::writeFileHeader"'?

wyldckat

2016-08-24 15:53

updater   ~0006745

diff --git a/src/functionObjects/forces/forceCoeffs/forceCoeffs.C b/src/functionObjects/forces/forceCoeffs/forceCoeffs.C
index 64d7251..cab68d6 100644
--- a/src/functionObjects/forces/forceCoeffs/forceCoeffs.C
+++ b/src/functionObjects/forces/forceCoeffs/forceCoeffs.C
@@ -60,9 +60,6 @@ void Foam::functionObjects::forceCoeffs::writeFileHeader(const label i)
         writeTabbed(file(i), "Cl");
         writeTabbed(file(i), "Cl(f)");
         writeTabbed(file(i), "Cl(r)");
- file(i)
- << tab << "Cm" << tab << "Cd" << tab << "Cl" << tab << "Cl(f)"
- << tab << "Cl(r)";
     }
     else if (i == 1)
     {

wyldckat

2016-08-24 15:53

updater   ~0006746

Sorry, it was quicker this way...

wyldckat

2016-08-24 16:25

updater  

wyldckat

2016-08-24 16:36

updater   ~0006748

Attached is the package "tweaks_for_a_few_function_objects_v2.tar.gz", which provides the following modified files for OpenFOAM-dev (in reference to the initial report):

Category A:
  src/functionObjects/field/fieldValues/fieldValueDelta/fieldValueDelta.C
  src/functionObjects/field/fieldValues/surfaceFieldValue/surfaceFieldValue.C
  src/functionObjects/field/fieldValues/volFieldValue/volFieldValue.C

Category B:
  src/functionObjects/forces/forceCoeffs/forceCoeffs.C
  src/functionObjects/forces/forces/forces.C
  src/functionObjects/forces/forces/forces.H


In review, the changes made to the "forces" library were as follows:

  - New enumeration "fileID", to help enforce which number refers to which file name.

  - Fixed typo in header for the very first header entry, where it was written "moment", but all others had "moments", so also change the first one to plural. And still on this header entry, changed from an explicit textual space to a separator with the variable 'tab'.

  - Some header text in "forces.C" was repeated and has been deferred to a "const word" for ensuring that "need only to be fixed once".

      - This refers to the strings "(pressure viscous porous)" and "[pressure,viscous,porous]" (for bins), which were repeated 4 times each.

      - I didn't change the ones for the bins to use "(pressure viscous porous)" as well, because the resulting "forces(1)[pressure,viscous,porous]" seems more readable than "forces(1)(pressure viscous porous)"... from a C++ point of view...

  - In both classes, the enumeration is now enforced for the most part (only exception is in "writeFileHeader", after confirming that the ID is valid), whenever it's necessary to write to a specific file.

      - This isn't bullet-proof, but the alternative requires using named-index lists (which was likely the original idea behind using hash-lists a few years ago).

henry

2016-08-24 19:36

manager   ~0006754

Thanks for these improvements Bruno

Resolved by commit b37269d481df77cc5996817900f17bc42a500867

Issue History

Date Modified Username Field Change
2016-08-24 15:46 wyldckat New Issue
2016-08-24 15:46 wyldckat Status new => assigned
2016-08-24 15:46 wyldckat Assigned To => henry
2016-08-24 15:46 wyldckat File Added: tweaks_for_a_few_function_objects_v1.tar.gz
2016-08-24 15:50 henry Note Added: 0006743
2016-08-24 15:53 wyldckat Note Added: 0006745
2016-08-24 15:53 wyldckat Note Added: 0006746
2016-08-24 16:25 wyldckat File Added: tweaks_for_a_few_function_objects_v2.tar.gz
2016-08-24 16:36 wyldckat Note Added: 0006748
2016-08-24 19:36 henry Note Added: 0006754
2016-08-24 19:36 henry Status assigned => resolved
2016-08-24 19:36 henry Fixed in Version => dev
2016-08-24 19:36 henry Resolution open => fixed