Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Patch to compile with Geant4 10.6 #803

Merged
merged 6 commits into from
Mar 9, 2023

Conversation

helen-brooks
Copy link
Collaborator

@helen-brooks helen-brooks commented Apr 6, 2022

Description

Patch to permit compilation with Geant4 10.6, solves #696

Motivation and Context

From the Geant4 10.6 Release notes:

  • The preprocessor macros G4UI_USE and G4VIS_USE are deprecated in 10.6 - these are used in /src/geant4/app/exampleN01.cc to test whether to include some headers.
  • added namespace scope to MeshScoreMap - the class MeshScoreMap has been moved inside the class G4VScoringMesh. The file ExN01UserScoreWriter.cc needs the statement
    using MeshScoreMap = G4VScoringMesh::MeshScoreMap;

Changes

  • Modify cmake files to check geant4 version, and define a preprocessor macro GEANT4_GT_10_6
  • Use preprocessor directive to include headers and define G4VScoringMesh alias in backwards-compatible way.

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....?

@helen-brooks helen-brooks changed the title Patch geant4 10 6 Patch to compile with Geant4 10.6 Apr 6, 2022
@helen-brooks helen-brooks requested a review from makeclean April 6, 2022 16:03
@gonuke
Copy link
Member

gonuke commented Apr 9, 2022

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
Copy link
Member

gonuke commented Apr 9, 2022

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

@helen-brooks
Copy link
Collaborator Author

helen-brooks commented Apr 11, 2022

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

I've not checked. Can do...

@gonuke
Copy link
Member

gonuke commented Apr 14, 2022

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
Copy link
Collaborator Author

helen-brooks commented Sep 16, 2022

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
Copy link
Member

gonuke commented Sep 16, 2022

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
Copy link
Collaborator Author

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

@ahnaf-tahmid-chowdhury
Copy link
Member

ahnaf-tahmid-chowdhury commented Feb 17, 2023

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
Copy link
Member

gonuke commented Feb 17, 2023

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
Copy link
Member

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
Copy link
Collaborator Author

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
Copy link
Member

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
Copy link
Collaborator Author

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

@gonuke
Copy link
Member

gonuke commented Mar 9, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants