View Issue Details

IDProjectCategoryView StatusLast Update
0001796OpenFOAMBugpublic2015-08-05 17:43
Reporterhk318i Assigned Tohenry  
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
PlatformGNU/LinuxOSUbuntuOS Version14.04
Product Versiondev 
Summary0001796: functionObjectFile OFstream list are not sorted corresponding to the input wordList
DescriptionI am working on project and I thought, it will be elegant and time saving solution to use functionObjectFile for writing to few output files. I tried the class and because it uses wordHashSet instead wordList, it wrote the data to the wrong file. Basically when I call file(i) or files[i], it does not return reference to OFstream which writes to file name(i) according to my input names list.

The problem appears only when the functionObjectFile class is used to write to more than two files. I reviewed quickly the source code and find out that OF function objects do not write to more that two files. Therefore it does not cause any harm now.

I know it is not a big issue and I managed to over come it in my project but I think it will be usful to modify it in the future or at least write a warning in the description.
Steps To ReproduceI am attaching here an incomplete implementation for forces library which writes every bin data to different file. I just modified createFileNames(), writeFileHeader() and commented out writeforcesBins() and writeBins() in the write() function. I was planing to modify writeforcesBins() and writeBins() as well but I think that is enough to show the problem for now.

Additional InformationThe output files

::::::::::::::
postProcessing/forces/0/forcesBins_bins_0.dat
::::::::::::::
# Force bins
# bin no --> check: 4
# x co-ords : 2.625237e-02 5.000475e-02 7.375712e-02 9.750950e-02
# y co-ords : 0.000000e+00 0.000000e+00 0.000000e+00 0.000000e+00
# z co-ords : 0.000000e+00 0.000000e+00 0.000000e+00 0.000000e+00
# Time forcesBins(0)[pressure,viscous,porous] moments(0)[pressure,viscous,porous] forcesBins(1)[pressure,viscous,porous] moments(1)[pressure,viscous,porous] forcesBins(2)[pressure,viscous,porous] moments(2)[pressure,viscous,porous] forcesBins(3)[pressure,viscous,porous] moments(3)[pressure,viscous,porous]
::::::::::::::
postProcessing/forces/0/forcesBins_bins_1.dat
::::::::::::::
# forcesBins
# CofR : (5.000000e-02 1.000000e-01 0.000000e+00)
# Time forcesBins(pressure viscous porous) moment(pressure viscous porous)
::::::::::::::
postProcessing/forces/0/forcesBins_bins_2.dat
::::::::::::::
# Force bins
# bin no --> check: 3
# x co-ords : 2.625237e-02 5.000475e-02 7.375712e-02 9.750950e-02
# y co-ords : 0.000000e+00 0.000000e+00 0.000000e+00 0.000000e+00
# z co-ords : 0.000000e+00 0.000000e+00 0.000000e+00 0.000000e+00
# Time forcesBins(0)[pressure,viscous,porous] moments(0)[pressure,viscous,porous] forcesBins(1)[pressure,viscous,porous] moments(1)[pressure,viscous,porous] forcesBins(2)[pressure,viscous,porous] moments(2)[pressure,viscous,porous] forcesBins(3)[pressure,viscous,porous] moments(3)[pressure,viscous,porous]
::::::::::::::
postProcessing/forces/0/forcesBins_bins_3.dat
::::::::::::::
# Force bins
# bin no --> check: 2
# x co-ords : 2.625237e-02 5.000475e-02 7.375712e-02 9.750950e-02
# y co-ords : 0.000000e+00 0.000000e+00 0.000000e+00 0.000000e+00
# z co-ords : 0.000000e+00 0.000000e+00 0.000000e+00 0.000000e+00
# Time forcesBins(0)[pressure,viscous,porous] moments(0)[pressure,viscous,porous] forcesBins(1)[pressure,viscous,porous] moments(1)[pressure,viscous,porous] forcesBins(2)[pressure,viscous,porous] moments(2)[pressure,viscous,porous] forcesBins(3)[pressure,viscous,porous] moments(3)[pressure,viscous,porous]
::::::::::::::
postProcessing/forces/0/forcesBins.dat
::::::::::::::
# Force bins
# bin no --> check: 1
# x co-ords : 2.625237e-02 5.000475e-02 7.375712e-02 9.750950e-02
# y co-ords : 0.000000e+00 0.000000e+00 0.000000e+00 0.000000e+00
# z co-ords : 0.000000e+00 0.000000e+00 0.000000e+00 0.000000e+00
# Time forcesBins(0)[pressure,viscous,porous] moments(0)[pressure,viscous,porous] forcesBins(1)[pressure,viscous,porous] moments(1)[pressure,viscous,porous] forcesBins(2)[pressure,viscous,porous] moments(2)[pressure,viscous,porous] forcesBins(3)[pressure,viscous,porous] moments(3)[pressure,viscous,porous]
TagsfunctionObject, Input/output, Post-processing

Activities

hk318i

2015-07-23 13:12

reporter  

forcesBins.tar.gz (8,905 bytes)

wyldckat

2015-07-23 14:16

updater   ~0005126

I was curious about this and took a look at the source code. This issue was introduced in OpenFOAM 2.2.0.
The reported problem is in the file "src/OpenFOAM/db/functionObjects/functionObjectFile/functionObjectFile.H", where this:

        //- File names
        wordHashSet names_;

Should indeed be this:

        //- File names
        wordList names_;

At a first glance, the hash list was probably used for performance reasons for looking up if there were duplicate names. And from your description, it seems that the "wordHashSet" class does not give the list of words in the same order as they were introduced.

Also, in the implementation that is implemented since 2.2.x, in the file "src/postProcessing/functionObjects/forces/forces/forces.C", the method "writeFileHeader" is relying on "i=0" to be the forces file and that all other files are for the bins. This is a considerable risk, given that the hashes will likely break the order of the name to file correspondence.


@hk318i: I haven't tested the code you've provided, but isn't the change of "names_" from "wordHashSet" to "wordList" in the file "functionObjectFile.H" enough to fix the problem?

hk318i

2015-07-23 14:34

reporter   ~0005127

@wyldckat: Yes, I think so but I am not sure what will be the implications of that. It requires some testing to confirm it. Also I recall that there is a function could sort the HashSet.
As you mentioned there is a potential risk, that's why I thought it worth reporting.

henry

2015-07-23 17:24

manager   ~0005130

I also do not see any particular need for the names to be stored as a wordHashSet particularly given that the files are stored as a PtrList. I would be happy for this code to be re-written using a wordList for names.

hk318i

2015-07-24 21:31

reporter  

hk318i

2015-07-24 21:31

reporter   ~0005137

I updated it and attached it here with small test app. The app is not the best test because I wrote it quickly in train today. However it could be useful for some users.

hk318i

2015-08-04 16:26

reporter  

functionObjectFile.patch (4,131 bytes)   
From f00de09e8a5397915e604f14aaa31f5620afb36e Mon Sep 17 00:00:00 2001
From: Hassan Kassem <hassan.kassem@gmail.com>
Date: Tue, 4 Aug 2015 15:53:13 +0100
Subject: [PATCH] functionObjectFile: Modified names type from wordHashSet to
 wordList Resolves bug-report
 http://www.openfoam.org/mantisbt/view.php?id=1796

---
 .../functionObjectFile/functionObjectFile.C          | 20 ++++++++------------
 .../functionObjectFile/functionObjectFile.H          |  4 ++--
 2 files changed, 10 insertions(+), 14 deletions(-)
 mode change 100644 => 100755 src/OpenFOAM/db/functionObjects/functionObjectFile/functionObjectFile.C
 mode change 100644 => 100755 src/OpenFOAM/db/functionObjects/functionObjectFile/functionObjectFile.H

diff --git a/src/OpenFOAM/db/functionObjects/functionObjectFile/functionObjectFile.C b/src/OpenFOAM/db/functionObjects/functionObjectFile/functionObjectFile.C
old mode 100644
new mode 100755
index 77a9b00..4f20adf
--- a/src/OpenFOAM/db/functionObjects/functionObjectFile/functionObjectFile.C
+++ b/src/OpenFOAM/db/functionObjects/functionObjectFile/functionObjectFile.C
@@ -85,8 +85,7 @@ void Foam::functionObjectFile::createFiles()
         const word startTimeName =
             obr_.time().timeName(obr_.time().startTime().value());
 
-        label i = 0;
-        forAllConstIter(wordHashSet, names_, iter)
+        forAll(names_, i)
         {
             if (!filePtrs_.set(i))
             {
@@ -94,7 +93,7 @@ void Foam::functionObjectFile::createFiles()
 
                 mkDir(outputDir);
 
-                word fName(iter.key());
+                word fName = names_[i];
 
                 // check if file already exists
                 IFstream is(outputDir/(fName + ".dat"));
@@ -109,7 +108,6 @@ void Foam::functionObjectFile::createFiles()
 
                 writeFileHeader(i);
 
-                i++;
             }
         }
     }
@@ -131,12 +129,12 @@ void Foam::functionObjectFile::write()
 void Foam::functionObjectFile::resetNames(const wordList& names)
 {
     names_.clear();
-    names_.insert(names);
+    names_.append(names);
 
     if (Pstream::master())
     {
         filePtrs_.clear();
-        filePtrs_.setSize(names_.toc().size());
+        filePtrs_.setSize(names_.size());
 
         createFiles();
     }
@@ -146,7 +144,7 @@ void Foam::functionObjectFile::resetNames(const wordList& names)
 void Foam::functionObjectFile::resetName(const word& name)
 {
     names_.clear();
-    names_.insert(name);
+    names_.append(name);
 
     if (Pstream::master())
     {
@@ -192,8 +190,7 @@ Foam::functionObjectFile::functionObjectFile
     filePtrs_()
 {
     names_.clear();
-    names_.insert(name);
-
+    names_.append(name);
     if (Pstream::master())
     {
         filePtrs_.clear();
@@ -217,8 +214,7 @@ Foam::functionObjectFile::functionObjectFile
     filePtrs_()
 {
     names_.clear();
-    names_.insert(names);
-
+    names_.append(names);
     if (Pstream::master())
     {
         filePtrs_.clear();
@@ -237,7 +233,7 @@ Foam::functionObjectFile::~functionObjectFile()
 
 // * * * * * * * * * * * * * * * Member Functions  * * * * * * * * * * * * * //
 
-const Foam::wordHashSet& Foam::functionObjectFile::names() const
+const Foam::wordList& Foam::functionObjectFile::names() const
 {
     return names_;
 }
diff --git a/src/OpenFOAM/db/functionObjects/functionObjectFile/functionObjectFile.H b/src/OpenFOAM/db/functionObjects/functionObjectFile/functionObjectFile.H
old mode 100644
new mode 100755
index 405e6eb..b3b64dd
--- a/src/OpenFOAM/db/functionObjects/functionObjectFile/functionObjectFile.H
+++ b/src/OpenFOAM/db/functionObjects/functionObjectFile/functionObjectFile.H
@@ -68,7 +68,7 @@ private:
         const word prefix_;
 
         //- File names
-        wordHashSet names_;
+        wordList names_;
 
         //- File pointer
         PtrList<OFstream> filePtrs_;
@@ -149,7 +149,7 @@ public:
     // Member Functions
 
         //- Return const access to the names
-        const wordHashSet& names() const;
+        const wordList& names() const;
 
         //- Return access to the file (if only 1)
         OFstream& file();
-- 
1.9.1

functionObjectFile.patch (4,131 bytes)   

henry

2015-08-05 17:42

manager   ~0005199

Thanks for the patch. I have updated OpenFOAM-dev:
commit fdd1beccf469faf4d396fa603dcc8a337b2a09da

Please re-open this report if there are any issues with this change.

Issue History

Date Modified Username Field Change
2015-07-23 13:12 hk318i New Issue
2015-07-23 13:12 hk318i File Added: forcesBins.tar.gz
2015-07-23 13:14 hk318i Tag Attached: functionObject
2015-07-23 13:14 hk318i Tag Attached: Input/output
2015-07-23 13:14 hk318i Tag Attached: Post-processing
2015-07-23 14:16 wyldckat Note Added: 0005126
2015-07-23 14:34 hk318i Note Added: 0005127
2015-07-23 17:24 henry Note Added: 0005130
2015-07-24 21:31 hk318i File Added: functionObjectFileTest.tar.gz
2015-07-24 21:31 hk318i Note Added: 0005137
2015-08-04 16:26 hk318i File Added: functionObjectFile.patch
2015-08-05 17:42 henry Note Added: 0005199
2015-08-05 17:42 henry Status new => resolved
2015-08-05 17:42 henry Resolution open => fixed
2015-08-05 17:42 henry Assigned To => henry