CFD Online Discussion Forums

CFD Online Discussion Forums (http://www.cfd-online.com/Forums/)
-   OpenFOAM Bugs (http://www.cfd-online.com/Forums/openfoam-bugs/)
-   -   List of glitches found on OpenFOAM 1.6 (http://www.cfd-online.com/Forums/openfoam-bugs/70066-list-glitches-found-openfoam-1-6-a.html)

wyldckat November 13, 2009 08:49

List of glitches found on OpenFOAM 1.6
 
Dear OpenFOAM developers,

This is more of a sum up of glitches, rather than a bug list (but this still looked to be the best place for this list), that were found while cross-compiling OpenFOAM using mingw32 and mingw-w64:
  1. When converting pointer addresses to integers, for address comparison, the best type cast is to use intptr_t instead of long or int, since pointer size is platform dependent.
    In OpenFOAM I only found one occurrence of address comparison in the file:
    Code:

    OpenFOAM-1.6/src/finiteVolume/fields/fvPatchFields/constraint/jumpCyclic/jumpCyclicFvPatchField.C
    Note: it is now fixed in 1.6.x git version.
  2. The Make folder for building libccmio is missing from the ThirdParty-1.6 package, since ThirdParty-1.5. Its contents are available here.
  3. The mpfr-2.4.1 folder that comes with the ThirdParty-1.6 package is missing 3 files from the original mpfr-2.4.1 package: log2.c, log1p.c and log10.c. I found out about this after trying to build mingw32 cross-compiler with gcc-4.3.3. I didn't check if this problem occurs with the Linux build of gcc-4.3.3.
  4. When it came to the phase of copying the tutorials from Linux to Windows, the only name clashes found were with the 2 tutorial cases for chtMultiRegionFoam, due to a file name clash of K and k. After some trial-and-errors of changing the files of the tutorial case
    Code:

    heatTransfer\chtMultiRegionFoam\multiRegionHeater
    I came to the conclusion that the files changeDictionaryDict have misleading indications that we can change the name of the field for K. For example:
    Code:

    bottomAir_to_leftSolid
    {
    type            solidWallMixedTemperatureCoupled;
    neighbourFieldName T;
    K              K;
    value          uniform 300;
    }

    Also looked at the code for the chtMultiRegionFoam executable, and I finally saw that it is hard-coded as field "K". That's why I called this more of a glitch, not quite a bug.
    So, is this a feature to be implemented in the future? Because I don't have enough experience with OpenFOAM's core code to retrieve the field name from the dictionary, and use it in the main field names.

Now, this is more of a request for the OpenFOAM 1.6.x: it is possible to build OpenFOAM successfully with gcc-4.4.? and gcc-4.5.0, just by using the info in the posts here and here. I used the following preprocessors, in the patches for mingw32 and mingw-w64, to insert the proper code corresponding to the targeted gcc version:
Code:

#if defined( __GNUC__ ) && defined( __GNUC_MINOR__ )
  #if (__GNUC__ == 4) && (__GNUC_MINOR__ >= 4)
  //... code for gcc-4.4.? and 4.5.0
  #else
  //code for gcc-4.3.? and below
  #endif
  #endif

Note: it is now fixed in 1.6.x git version.

I hope this helps, or at the very least can be a future reference for others that may encounter the same glitches.

Best Regards,
Bruno

mwild November 15, 2009 04:46

Although I'm not an official OpenFOAM developer, here my thoughts:

Quote:

Originally Posted by wyldckat (Post 236134)
When converting pointer addresses to integers, for address comparison, the best type cast is to use intptr_t instead of long or int, since pointer size is platform dependent.
  1. In OpenFOAM I only found one occurrence of address comparison in the file:
    Code:

    OpenFOAM-1.6/src/finiteVolume/fields/fvPatchFields/constraint/jumpCyclic/jumpCyclicFvPatchField.C

Trouble is, intptr_t is only required in C99 and should probably will be adopted in the next C++ standard. And obviously there are systems around where stdint.h does not contain a definition for intptr_t.
Quote:

Originally Posted by wyldckat (Post 236134)
  1. When it came to the phase of copying the tutorials from Linux to Windows, the only name clashes found were with the 2 tutorial cases for chtMultiRegionFoam, due to a file name clash of K and k. After some trial-and-errors of changing the files of the tutorial case
    Code:

    heatTransfer\chtMultiRegionFoam\multiRegionHeater
    I came to the conclusion that the files changeDictionaryDict have misleading indications that we can change the name of the field for K. For example:
    Code:

    bottomAir_to_leftSolid
    {
    type            solidWallMixedTemperatureCoupled;
    neighbourFieldName T;
    K              K;
    value          uniform 300;
    }

    Also looked at the code for the chtMultiRegionFoam executable, and I finally saw that it is hard-coded as field "K". That's why I called this more of a glitch, not quite a bug.
    So, is this a feature to be implemented in the future? Because I don't have enough experience with OpenFOAM's core code to retrieve the field name from the dictionary, and use it in the main field names.

Are you sure the only conflict is k and K? If I remember correctly, there's also a conflict between b and B... Although there is no conflict in the current tutorials, it stands to reason that somebody will want to do combustion under influence of a magnetic field.

The names of the files are usually hard-coded in the createFields.H files and cannot be configured. changeDictionary allows you to change a dictionary's content, e.g. to change a boundary condition.

Quote:

Originally Posted by wyldckat (Post 236134)
Now, this is more of a request for the OpenFOAM 1.6.x: it is possible to build OpenFOAM successfully with gcc-4.4.? and gcc-4.5.0, just by using the info in the posts here and here. I used the following preprocessors, in the patches for mingw32 and mingw-w64, to insert the proper code corresponding to the targeted gcc version:
Code:

#if defined( __GNUC__ ) && defined( __GNUC_MINOR__ )
  #if (__GNUC__ == 4) && (__GNUC_MINOR__ >= 4)
  //... code for gcc-4.4.? and 4.5.0
  #else
  //code for gcc-4.3.? and below
  #endif
  #endif


These patches will AFAIK work perfectly with older GCC compilers and do not need such atrocious preprocessor distiction.

Michael

wyldckat November 15, 2009 11:20

Great learning opportunity!
 
Hello Michael,

Thank you very much for your reply!

Quote:

Originally Posted by mwild (Post 236294)
Trouble is, intptr_t is only required in C99 and should probably will be adopted in the next C++ standard. And obviously there are systems around where stdint.h does not contain a definition for intptr_t.

I actually knew that intptr_t was only introduced with C99, but after so many rounds trying to get gcc+Mingw-w64 to build properly (caught the Mingw-w64 project just before a preprocessor fix for intptr_t), I felt that this macro always came along with gcc to every architecture, at the very least since gcc-4.3.3, so I thought that this could be used independent of the C99.

Quote:

Originally Posted by mwild (Post 236294)
Are you sure the only conflict is k and K? If I remember correctly, there's also a conflict between b and B... Although there is no conflict in the current tutorials, it stands to reason that somebody will want to do combustion under influence of a magnetic field.
The names of the files are usually hard-coded in the createFields.H files and cannot be configured.

Thank you very much! I was afraid that such could happen, but I currently don't fully grasp OpenFOAM's core. I'm making a note of this, because cross-compiling to Windows will require code adaptations for those variables! And the usual suspect files is a very good start! It'll save a lot of time for me and others who want to port to an case insensitive file system.

Quote:

Originally Posted by mwild (Post 236294)
changeDictionary allows you to change a dictionary's content, e.g. to change a boundary condition.

What really got me here, is the fact that the dictionary files hinted to something that wasn't true... and it took me some time to figure out that they didn't do as I thought they would :(

Quote:

Originally Posted by mwild (Post 236294)
These patches will AFAIK work perfectly with older GCC compilers and do not need such atrocious preprocessor distiction.

I feel obligated to humbly apologize for proposing such an inelegant solution. It crossed my mind to just leave the code there for all solutions, but at the time I did the patches, I didn't want to waste more time testing with multiple configurations.

In the last few hours, I git pulled the latest version, and built the full Linux x86_64 version, to figure out if 1.6.x already had these fixed. Due to your comment, I felt that I might have missed the fixes in 1.6.x, and that I might have made false accusations. I'm happy to report that all of OpenFOAM built nice and clean with gcc-4.4.2. And after analyzing the 1.6.x git tree with gitk, I saw that the issues about intptr_t and gcc-4.4.? and gcc-4.5.0 were already fixed, on the same day of my post, in a highly elegant coding fashion, that brings me to the self-awareness that I'm nothing more than still a young coding grass-hopper.

Once again, thank you Michael! And I'm glad that I could help, even with my limited knowledge and experience.

Best regards,
Bruno

mwild November 15, 2009 14:04

I didn't want to give the impression that I'm an experienced OpenFOAM-developer; I'm certainly not... I just saw your post with many good points (such as the missing files to build ccmio) and thought I'd point out some issues that popped to my mind.

Michael

mattijs November 17, 2009 08:29

I've pushed the 'fixes' to make 1.6.x compile with gcc 4.4 series.

Thanks for reporting,

Mattijs

wyldckat November 17, 2009 08:51

Thank you and you're welcome! :)

I already tested it on Sunday, with gcc-4.4.2 (Ubuntu 9.04 x86_64), and everything compiled with no problems. Haven't tested these with gcc-4.5.0, but I believe that it should work well with it too, because the fixes indicated in the other thread worked well with 4.5.0.

Best Regards,
Bruno


All times are GMT -4. The time now is 08:19.