GRIFFINCollaboration / detectorSimulations_v10

Geant4 version 10 of the simulation code for the GRIFFIN array and it's suite of ancillary detection systems.
MIT License
9 stars 14 forks source link

Errors in CMakeLists.txt file (Geant4.10.02, detectorSimulations_v10) #79

Closed hrobertson2 closed 1 year ago

hrobertson2 commented 1 year ago

I am trying to compile detectorSimulations _v10 with cmake, but am getting an error:

CMake Error at CMakeLists.txt:57 (include): include called with wrong number of arguments. include() only takes one file.

I am using geant4.10.02.

Thank you!

VinzenzBildstein commented 1 year ago

Which cmake version are you using and which version and branch of detectorSimulations_v10 are you using?

VinzenzBildstein commented 1 year ago

I've just tested branch geant4.10.04 with cmake 3.22.1 on ubuntu 22.04 and cmake 3.20.2 on CentOS 8.5 and both work without that error. This was using geant4.10.06.p2.

hrobertson2 commented 1 year ago

The cmake version is 3.24.3 and I believe that I am using the 1.0.0 version of detectorSimulations_v10. Is there a way to check this?

VinzenzBildstein commented 1 year ago

You can use git status to see the branch you are on, and git log -1 to see the latest commit. Could you also post the result of root-config --version here ?

hrobertson2 commented 1 year ago

Using git status I see that I am on branch main.

And the output for the latest commit is: commit 8d66d7fce529e1606ed288e7bee1576acb05aa6b Merge: e6582e8 e5b0151 Author: Vinzenz Bildstein vbildste@uoguelph.ca Date: Tue Jul 5 11:53:24 2022 -0400

The root version is 6.10/04.

VinzenzBildstein commented 1 year ago

Can you try the geant4.10.04 branch instead? I think that branch is the more recent one. Not sure if it works with gean4.10.02, but worth a try

hrobertson2 commented 1 year ago

I've tried using the geant4.10.04 branch and am still getting the same error (though on line 58 now)

smayotte commented 1 year ago

I have the same issue. The problem (for me) is with the line include(${ROOT_USE_FILE}) as ${ROOT_USE_FILE} seems to be empty. If I comment that line cmake works, but I'm not sure if doing that breaks anything else.

VinzenzBildstein commented 1 year ago

Can you try that same solution @hrobertson2 ?

hrobertson2 commented 1 year ago

cmake did work after commenting out the line that was suggested. However, I'm now encountering some errors in running make. They all seem to have to do with the DetectionSystemTrific.cc file. Did you also have this issue @smayotte?

VinzenzBildstein commented 1 year ago

What are the error messages, and which version of the code is this happening with?

hrobertson2 commented 1 year ago

Errors:

/home/hrobertson2/packages/detectorSimulations_v10/src/DetectionSystemTrific.cc: In constructor ‘DetectionSystemTrific::DetectionSystemTrific(G4double, G4double, G4bool, G4bool, G4double, G4String)’: /home/hrobertson2/packages/detectorSimulations_v10/src/DetectionSystemTrific.cc:60:16: error: ‘STP_Temperature’ was not declared in this scope temperature = STP_Temperature; ^ /home/hrobertson2/packages/detectorSimulations_v10/src/DetectionSystemTrific.cc: In constructor ‘DetectionSystemTrific::DetectionSystemTrific(G4double, G4double, G4bool, G4bool, G4double, G4String)’: /home/hrobertson2/packages/detectorSimulations_v10/src/DetectionSystemTrific.cc:60:16: error: ‘STP_Temperature’ was not declared in this scope temperature = STP_Temperature; ^

I am running the geant4.10.04 branch.

And the latest commit info is: commit 69c02dcc65edac069aa3cc88c60ad8ad905633d7 Merge: bf06110 e6582e8 Author: VinzenzBildstein vbildste@uoguelph.ca Date: Mon Aug 30 11:38:59 2021 -0400

smayotte commented 1 year ago

cmake did work after commenting out the line that was suggested. However, I'm now encountering some errors in running make. They all seem to have to do with the DetectionSystemTrific.cc file. Did you also have this issue @smayotte?

No I did not have that particular issue, but some other ones. I was running with geant4.10.02 as that is recommended from the README. I am using the main branch right now. What is the recommended c++ standard for detectorSimulations_v10? I was getting errors when not using C++ 17. Now with using C++ 17 I still get this error:

/home/apollo/Research/projects/NeutronExp/code/detectorSimulations_v10_git/src/PhysListHadron.cc: In member function ‘virtual void PhysListHadron::ConstructProcess()’:
/home/apollo/Research/projects/NeutronExp/code/detectorSimulations_v10_git/src/PhysListHadron.cc:141:9: warning: ‘auto(x)’ only available with ‘-std=c++2b’ or ‘-std=gnu++2b’ [-Wc++23-extensions]
  141 |         auto aParticleIterator = GetParticleIterator();
      |         ^
/home/apollo/Research/projects/NeutronExp/code/detectorSimulations_v10_git/src/PhysListHadron.cc:141:9: error: lvalue required as left operand of assignment
make[2]: *** [CMakeFiles/Griffinv10.dir/build.make:538: CMakeFiles/Griffinv10.dir/src/PhysListHadron.cc.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:83: CMakeFiles/Griffinv10.dir/all] Error 2
make: *** [Makefile:136: all] Error 2

If I set the CXX standard to C++23 this error remains:

/home/apollo/Research/projects/NeutronExp/code/detectorSimulations_v10_git/src/PhysListHadron.cc:141:9: error: lvalue required as left operand of assignment
  141 |         auto aParticleIterator = GetParticleIterator();
      |         ^
make[2]: *** [CMakeFiles/Griffinv10.dir/build.make:538: CMakeFiles/Griffinv10.dir/src/PhysListHadron.cc.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:83:

For what it's worth, it all compiles fine when I'm using geant4.10.04 (also on the main branch) but then doesn't run. I haven't tested the geant4.10.04 branch yet.

smayotte commented 1 year ago

Errors:

/home/hrobertson2/packages/detectorSimulations_v10/src/DetectionSystemTrific.cc: In constructor ‘DetectionSystemTrific::DetectionSystemTrific(G4double, G4double, G4bool, G4bool, G4double, G4String)’: /home/hrobertson2/packages/detectorSimulations_v10/src/DetectionSystemTrific.cc:60:16: error: ‘STP_Temperature’ was not declared in this scope temperature = STP_Temperature; ^ /home/hrobertson2/packages/detectorSimulations_v10/src/DetectionSystemTrific.cc: In constructor ‘DetectionSystemTrific::DetectionSystemTrific(G4double, G4double, G4bool, G4bool, G4double, G4String)’: /home/hrobertson2/packages/detectorSimulations_v10/src/DetectionSystemTrific.cc:60:16: error: ‘STP_Temperature’ was not declared in this scope temperature = STP_Temperature; ^

I am running the geant4.10.04 branch.

And the latest commit info is: commit 69c02dc Merge: bf06110 e6582e8 Author: VinzenzBildstein vbildste@uoguelph.ca Date: Mon Aug 30 11:38:59 2021 -0400

If you use the DetectionSystemTrific.cc file from the main branch it might compile, at least that just worked for me.

hrobertson2 commented 1 year ago

Thank you @smayotte! That did solve that issue, but now I am getting the same errors as you are:

In file included from /home/hrobertson2/packages/detectorSimulations_v10/include/PhysListHadron.hh:36:0, from /home/hrobertson2/packages/detectorSimulations_v10/src/PhysListHadron.cc:32: /home/hrobertson2/packages/detectorSimulations_v10/src/PhysListHadron.cc: In member function ‘virtual void PhysListHadron::ConstructProcess()’: /home/hrobertson2/packages/detectorSimulations_v10/src/PhysListHadron.cc:141:7: error: invalid use of ‘auto’ auto aParticleIterator = GetParticleIterator(); ^ /home/hrobertson2/packages/detectorSimulations_v10/src/PhysListHadron.cc:141:47: error: ‘GetParticleIterator’ was not declared in this scope auto aParticleIterator = GetParticleIterator(); ^ make[2]: *** [CMakeFiles/Griffinv10.dir/src/PhysListHadron.cc.o] Error 1 make[1]: *** [CMakeFiles/Griffinv10.dir/all] Error 2 make: *** [all] Error 2

VinzenzBildstein commented 1 year ago

Okay, all these errors sound to me like some fixes have been implemented for some branches and not others. I haven't tested the code with older geant4 versions in a while. I currently use geant4.10.06.p02 and the geant4.10.04 branch with some modifications:

In addition I got an email from Joey Turko saying that for him the solution was to add a line "list(APPEND CMAKE_PREFIX_PATH $ENV{ROOTSYS})" to CMakeLists.txt.

The "recommendation" of using geant4.10.02 isn't quite that much of a recommendation I think, it's more that someone took the time to check that the results were good, and since then no-one has bothered doing so with the newer versions. This could be quite helpful, as so far we need to use geant4.10.01 with Evan's patch to get angular correlations to work, which should be integrated without need for a home-made patch in the newer versions (see also #40). So at some point someone will need to compare an angular correlation simulation for the old code with the new version and see if they agree (or simply check that the new versions gives the expected results). Until that is done we can't really recommend the newer versions I think? Of course that doesn't mean they can't be used.

smayotte commented 1 year ago

I was only having issues compiling the code with geant4.10.02. With geant4.10.04 compiling works fine for both the geant4.10.04 branch and the main branch. The run.mac is missing /run/initialize at the start btw, which at least made running a simulation break for me, but adding the initialization fixed things.

Would you recommend using the geant4.10.04 over the main branch in that case?

VinzenzBildstein commented 1 year ago

Yes, I think I would recommend using geant4.10.04, keeping in mind that some things haven't been tested with that version.

It's curious that the main branch works with geant4.10.04, as in there were some changes in the code necessary to make it run with 10.04. Maybe there was an accidental merge at some point the main branch is the same as the geant4.10.04 branch now? I will have to look into this.

EDIT: According to #73, this is the case, we now have a main branch that is meant for geant4.10.04, and the old main branch is now called angular-correlations. So I guess I will have to correct the README. This also means that geant4.10.04 is now the recommended version.

hrobertson2 commented 1 year ago

Thank you @VinzenzBildstein and @smayotte! I ended up just downloading geant4.10.04.p03 as it seemed like that would be the easiest route. Everything compiled correctly.

VinzenzBildstein commented 1 year ago

Thanks for confirming that geant4.10.04 with the main branch works. I've updated the README accordingly. I think we can close the issue now?