Problem 1846

Summary: G4MTRandom incorrect if statement
Product: Geant4 Reporter: Henry Schreiner <henryschreineriii>
Component: globalAssignee: Gabriele Cosmo <Gabriele.Cosmo>
Status: RESOLVED INVALID    
Severity: major    
Priority: P5    
Version: 10.2   
Hardware: PC   
OS: All   

Description Henry Schreiner 2016-03-15 23:08:04 CET
I’ve discovered an error in the Multithreaded source for Geant4, at least I think it is. When I build with multithreading enabled on linux (TACC’s Lonestar5), Geant4MT works fine unless I try to access the random number generator, when I get a symbol missing (G4MTHepRandom getTheSeed). The exact same code works perfectly, however, on my mac (with 10.1.02). Upon further investigation, I found that the multithreading components of the random number generator seem to get skipped when building on linux, but some of the other portions of the multithreading get built properly. 

I believe all settings are the same, and both are installing with GCC (5.2 in both cases). The same thing on the previous linux computer (Lodestar, Geant 10.0, CMake 2.8) worked, as well, giving similar output to the mac. I’ve tried three builds, 10.2, 10.2.1 (patched), and 10.2.1. It is building the files, I see source/global/CMakeFiles/G4global.dir/HEPRandom/src/G4MTHepRandom.cc.o in the make output. However, it has next to no symbols in it. This makes me think that there is something wrong with the line:

#if __clang__
  #if ((defined(G4MULTITHREADED) && !defined(G4USE_STD11)) || \
      !__has_feature(cxx_thread_local))
    #define CLANG_NOSTDTLS
  #endif
#endif

#if (defined(G4MULTITHREADED) && \
    (!defined(G4USE_STD11) || (defined(CLANG_NOSTDTLS) || defined(__INTEL_COMPILER))))

Also, why are these always built if c++11 is missing? And isn’t the USE_STD11 unneeded now that 10.2 requires c++11+? The CMake file says it’s only defined for backward compatibility. Also, 10.0 did not have this if statement at all.

I would propose that those lines are all changed to something like:

#if (defined(G4MULTITHREADED))

or

#if __clang__
  #if ((defined(G4MULTITHREADED) && !defined(G4USE_STD11)) || \
      !__has_feature(cxx_thread_local))
    #define CLANG_NOSTDTLS
  #endif
#endif

#if (defined(G4MULTITHREADED) && \
    !((defined(CLANG_NOSTDTLS) || defined(__INTEL_COMPILER))))

This, I think, works as expected, with the first one working as long as CMake correctly identifies the intel and clang compilers that do not support these features - which is a better method than having undefined symbols on some systems, IMO. It seems to build correctly for me if I make that change. This change/problem is for every file in global/HEPRandom/src and global/HEPRandom/include/Randomize.hh.
Comment 1 Gabriele Cosmo 2016-03-15 23:32:30 CET
The G4MT wrappers for random numbers are -only- used in MT mode for compilers NOT fully supporting the C++11 standard.
All compilers fully supporting the c++11 Standard (or for sequential builds) will directly use the CLHEP code as it is (wrappers are unused).
This is valid only starting from release 10.2 (the patch fixes a configuration issue for gcc-5).
You should use G4Random::getTheSeed() in your code.
Comment 2 Henry Schreiner 2016-03-15 23:48:54 CET
That is exactly what I do use ( G4Random::getTheSeed() ). And it tries to refer to that symbol. In sequential mode it compiles and works fine, as it does if that if statement is removed.

I assume I still include 

#include "G4MTRunManager.hh"
#include "G4Threading.hh"
#else
#include "G4RunManager.hh"

? That is the only change I make for multithreaded vs. sequential, besides using the G4MTRunManager class and setting the number of threads.
Comment 3 Gabriele Cosmo 2016-03-15 23:53:33 CET
It doesn't appear so to me. Using gcc-5.3, G4Random is typedef to HEPRandom (i.e. CLHEP).
See Randomize.hh.
Comment 4 Henry Schreiner 2016-03-16 00:02:41 CET
That is really odd, I see what you are referring to. But somehow it is picking up G4MTHepRandom. I'll undo the patch and retry after a few jobs I'm running finish, to see why it is finding G4MTHepRandom. I don't have the string "Random" anywhere save for randomize.hh.

Thanks for your help, I'll let you know if I work it out to being some odd header or something.
Comment 5 Henry Schreiner 2016-03-16 00:39:39 CET
 I believe I found the problem. Since that was in a header, I believe that I was not adding the  G4USE_STD11 definition to my compilation,  thereby causing it to include the wrong one. I can fix mine to match the GEANT flags, but I would suggest removing the G4USE_STD11  portion of the statement, since it is now set always, and listed as for back compatibility only.

Thanks for the assist! Sorry for the false alarm.
Comment 6 Henry Schreiner 2016-03-16 02:52:44 CET
I checked, and ${Geant4_DEFINITIONS} from findpackage(Geant4) does not include -DG4USE_STD11, so that must be why there was a mismatch between header and library.

For now, I can add that to the definition list manually.