svalinn / DAGMC

Direct Accelerated Geometry Monte Carlo Toolkit
https://svalinn.github.io/DAGMC
Other
97 stars 63 forks source link

Patch to compile with Geant4 10.6 #803

Closed helen-brooks closed 1 year ago

helen-brooks commented 2 years ago

Description

Patch to permit compilation with Geant4 10.6, solves #696

Motivation and Context

From the Geant4 10.6 Release notes:

Changes

Behavior

DAG-Geant4 now compiles with Geant4 10.6

Other Information

I haven't touched the CI. I don't know what you want to do about this - presumably it's overkill to add yet another variation to the CI matrix....?

gonuke commented 2 years ago

While I agree it's overkill to add another variation to the CI matrix, we may want to upgrade to 10.6 now that this works.

gonuke commented 2 years ago

Related question - do we know if this works with newer versions? 10.7 or 11.0?

helen-brooks commented 2 years ago

Related question - do we know if this works with newer versions? 10.7 or 11.0?

I've not checked. Can do...

gonuke commented 2 years ago

Related question - do we know if this works with newer versions? 10.7 or 11.0?

I've not checked. Can do...

It's fine if not, but if yes, it would be good to upgrade CI & docs to newest version we support.

helen-brooks commented 2 years ago

Hello - little nudge on this patch for building with Geant4 10.6. Were there any changes needed from my side? (Aside from the obvious do a rebase and resolve any conflicts). I ask because the student I'm working with who is using this patch for their work is now nearing the end of their project, only a few months left, it'd be nice to get this in. @makeclean

gonuke commented 2 years ago

I guess I was waiting for a quick assessment on whether it was possible to jump all the way to a higher GEANT4 version while we're at it. I'm not looking to make a bunch of work, but just hopeful that your improvements might simply apply to a higher version as well.

If you can confirm that this is NOT the case, I'm happy to move forward.

helen-brooks commented 2 years ago

No problem, I'll try and get to this next week :)

ahnaf-tahmid-chowdhury commented 1 year ago

I have got this while using forked version:

[ 21%] Building CXX object src/uwuw/tests/CMakeFiles/uwuw_unit_tests.dir/uwuw_unit_tests.cpp.o
/opt/nuclear-boy/dagmc-repo-forked/src/uwuw/tests/uwuw_unit_tests.cpp: In member function ‘virtual void UWUWTest_filepath2_Test::TestBody()’:
/opt/nuclear-boy/dagmc-repo-forked/src/uwuw/tests/uwuw_unit_tests.cpp:47:9: warning: ignoring return value of ‘char* getcwd(char*, size_t)’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
   47 |   getcwd(current_path, sizeof(current_path));
      |   ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/opt/nuclear-boy/dagmc-repo-forked/src/uwuw/tests/uwuw_unit_tests.cpp: In member function ‘virtual void UWUWTest_filepath3_Test::TestBody()’:
/opt/nuclear-boy/dagmc-repo-forked/src/uwuw/tests/uwuw_unit_tests.cpp:62:9: warning: ignoring return value of ‘char* getcwd(char*, size_t)’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
   62 |   getcwd(current_path, sizeof(current_path));
      |   ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/opt/nuclear-boy/dagmc-repo-forked/src/uwuw/tests/uwuw_unit_tests.cpp: In member function ‘virtual void UWUWTest_filepath4_Test::TestBody()’:
/opt/nuclear-boy/dagmc-repo-forked/src/uwuw/tests/uwuw_unit_tests.cpp:78:9: warning: ignoring return value of ‘char* getcwd(char*, size_t)’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
   78 |   getcwd(current_path, sizeof(current_path));
      |   ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[ 21%] Building CXX object src/uwuw/tests/CMakeFiles/uwuw_unit_tests.dir/uwuw_unit_test_driver.cpp.o
[ 22%] Linking CXX executable uwuw_unit_tests
/usr/bin/ld: CMakeFiles/uwuw_unit_tests.dir/uwuw_unit_tests.cpp.o: in function `UWUWTest_mat_write_Test::TestBody()':
uwuw_unit_tests.cpp:(.text+0x2364): undefined reference to `pyne::Material::openmc(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, int)'
collect2: error: ld returned 1 exit status
make[2]: *** [src/uwuw/tests/CMakeFiles/uwuw_unit_tests.dir/build.make:124: src/uwuw/tests/uwuw_unit_tests] Error 1
make[1]: *** [CMakeFiles/Makefile2:809: src/uwuw/tests/CMakeFiles/uwuw_unit_tests.dir/all] Error 2
make: *** [Makefile:146: all] Error 2
Consolidate compiler generated dependencies of target gtest
[  1%] Built target gtest
Consolidate compiler generated dependencies of target dagmc-shared
[  3%] Built target dagmc-shared
Consolidate compiler generated dependencies of target pt_vol_test
[  4%] Built target pt_vol_test
Consolidate compiler generated dependencies of target ray_fire_test
[  5%] Built target ray_fire_test
Consolidate compiler generated dependencies of target test_geom
[  6%] Built target test_geom
Consolidate compiler generated dependencies of target dagmc_unit_tests
[  8%] Built target dagmc_unit_tests
Consolidate compiler generated dependencies of target dagmc_pointinvol_test
[ 10%] Built target dagmc_pointinvol_test
Consolidate compiler generated dependencies of target dagmc_rayfire_test
[ 12%] Built target dagmc_rayfire_test
Consolidate compiler generated dependencies of target dagmc_simple_test
[ 14%] Built target dagmc_simple_test
Consolidate compiler generated dependencies of target pyne_dagmc-shared
[ 16%] Built target pyne_dagmc-shared
Consolidate compiler generated dependencies of target uwuw-shared
[ 19%] Built target uwuw-shared
Consolidate compiler generated dependencies of target uwuw_preproc
[ 20%] Built target uwuw_preproc
Consolidate compiler generated dependencies of target uwuw_unit_tests
[ 21%] Linking CXX executable uwuw_unit_tests
/usr/bin/ld: CMakeFiles/uwuw_unit_tests.dir/uwuw_unit_tests.cpp.o: in function `UWUWTest_mat_write_Test::TestBody()':
uwuw_unit_tests.cpp:(.text+0x2364): undefined reference to `pyne::Material::openmc(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, int)'
collect2: error: ld returned 1 exit status
make[2]: *** [src/uwuw/tests/CMakeFiles/uwuw_unit_tests.dir/build.make:124: src/uwuw/tests/uwuw_unit_tests] Error 1
make[1]: *** [CMakeFiles/Makefile2:809: src/uwuw/tests/CMakeFiles/uwuw_unit_tests.dir/all] Error 2
make: *** [Makefile:146: all] Error 2

The error occurred during the linking stage of the compilation process. Specifically, the linker could not find a reference to the function pyne::Material::openmc() which is called in the file uwuw_unit_tests.cpp. This means that the linker was not able to locate the library or object file that contains the definition for the pyne::Material::openmc() function.

gonuke commented 1 year ago

Alas, the forked version may need to be rebased on the current develop branch to capture more recent updates. This is not a Geant related issue, so suggests that this branch is a little behind necessary updates in develop.

ahnaf-tahmid-chowdhury commented 1 year ago

Dear @helen-brooks, I have recently created a PR #860 which solves the same issue. It would be great if we work together. I have also made a PR to your repo.

However, I am wondering to know is it worthful working with the old versions of Gent4 < 10.6. As they are outdated and people may use the latest versions with latest DAGMC.

helen-brooks commented 1 year ago

Hi all. I ran a bunch of jobs and my experience is that with this change dagmc builds and Geant4 tests pass for all these versions: 10.4.3 10.5.0 10.5.1 10.6.0 10.6.1 10.6.2 10.6.3 10.7.0 10.7.1 10.7.2 10.7.3 10.7.4

After that, namely for the tag version 11.0.0, I get this error:

/home/ir-broo2/rds/rds-ukaea-ap001/ir-broo2/test-dagmc-geant4-patch/dagmc/DAGMC/src/geant4/app/include/ExN01Analysis.hh:10:10: fatal error: g4root.hh: No such
 file or directory
   10 | #include "g4root.hh"
      |          ^~~~~~~~~~~
compilation terminated.
make[2]: *** [src/geant4/app/CMakeFiles/DagGeant4.dir/src/ExN01EventAction.cc.o] Error 1

Sorry for taking so long to check this.

ahnaf-tahmid-chowdhury commented 1 year ago

It seems like the issue you are facing is related to the use of g4root.hh in version 11.0.0 of Geant4, which has been replaced with G4AnalysisManager.hh. I have fixed this issue in my pull request by updating the code to use the new header file.

helen-brooks commented 1 year ago

@gonuke Given that I've now tested the compatibility, are there any further changes you require?

gonuke commented 1 year ago

Thanks @helen-brooks this should be good to go.