search

svalinn / DAGMC

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

Updated DAGMC code to be compatible with Geant4 v11.1.1 #860

Closed ahnaf-tahmid-chowdhury closed 11 months ago

ahnaf-tahmid-chowdhury commented 1 year ago

Description

In this pull request, I have updated the DAGMC code to be compatible with Geant4 v11.1.1. This involved making changes to exampleN01.cc , ExN01Analysis.hh, ExN01DetectorConstruction.cc, ExN01UserScoreWriter.cc and generate_geant4 which previously were only compatible with Geant4 v10.5.

Motivation and Context

This change is required because many users are currently using Geant4 v11.1.1 and need DAGMC to be compatible with it. The previous version of DAGMC was causing issues for users attempting to use it with this version of Geant4, as described in issue #857.

Changes

This PR introduces a bug fix, updating the DAGMC code to be compatible with Geant4 v11.1.1.

Behavior

Previously, DAGMC was only compatible with Geant4 v10.5. This pull request updates the code to be compatible with Geant4 v11.1.1, allowing users to use DAGMC with this newer version of Geant4.

Other Information

I have tested these changes and verified that they work correctly. 

./dagsolid_unit_tests
[==========] Running 16 tests from 1 test case.
[----------] Global test environment set-up.
[----------] 16 tests from DagSolidTest
[ RUN      ] DagSolidTest.SetUp
Set overlap thickness = 0
Set numerical precision = 0.001
Loading file test_geom.h5m
Initializing the GeomQueryTool...
Using faceting tolerance: 0.0001
Building acceleration data structures...
[       OK ] DagSolidTest.SetUp (92 ms)
[ RUN      ] DagSolidTest.point_in_volume
Set overlap thickness = 0
Set numerical precision = 0.001
Loading file test_geom.h5m
Initializing the GeomQueryTool...
Using faceting tolerance: 0.0001
Building acceleration data structures...
[       OK ] DagSolidTest.point_in_volume (34 ms)
[ RUN      ] DagSolidTest.point_on_surface
Set overlap thickness = 0
Set numerical precision = 0.001
Loading file test_geom.h5m
Initializing the GeomQueryTool...
Using faceting tolerance: 0.0001
Building acceleration data structures...
[       OK ] DagSolidTest.point_on_surface (35 ms)
[ RUN      ] DagSolidTest.point_out_outside_tolerance
Set overlap thickness = 0
Set numerical precision = 0.001
Loading file test_geom.h5m
Initializing the GeomQueryTool...
Using faceting tolerance: 0.0001
Building acceleration data structures...
[       OK ] DagSolidTest.point_out_outside_tolerance (34 ms)
[ RUN      ] DagSolidTest.point_out_surface_tolerance
Set overlap thickness = 0
Set numerical precision = 0.001
Loading file test_geom.h5m
Initializing the GeomQueryTool...
Using faceting tolerance: 0.0001
Building acceleration data structures...
[       OK ] DagSolidTest.point_out_surface_tolerance (34 ms)
[ RUN      ] DagSolidTest.point_on_surface_tolerance
Set overlap thickness = 0
Set numerical precision = 0.001
Loading file test_geom.h5m
Initializing the GeomQueryTool...
Using faceting tolerance: 0.0001
Building acceleration data structures...
[       OK ] DagSolidTest.point_on_surface_tolerance (35 ms)
[ RUN      ] DagSolidTest.point_outside_volume
Set overlap thickness = 0
Set numerical precision = 0.001
Loading file test_geom.h5m
Initializing the GeomQueryTool...
Using faceting tolerance: 0.0001
Building acceleration data structures...
[       OK ] DagSolidTest.point_outside_volume (34 ms)
[ RUN      ] DagSolidTest.test_2
Set overlap thickness = 0
Set numerical precision = 0.001
Loading file test_geom.h5m
Initializing the GeomQueryTool...
Using faceting tolerance: 0.0001
Building acceleration data structures...
[       OK ] DagSolidTest.test_2 (34 ms)
[ RUN      ] DagSolidTest.test_3
Set overlap thickness = 0
Set numerical precision = 0.001
Loading file test_geom.h5m
Initializing the GeomQueryTool...
Using faceting tolerance: 0.0001
Building acceleration data structures...
[       OK ] DagSolidTest.test_3 (34 ms)
[ RUN      ] DagSolidTest.test_4
Set overlap thickness = 0
Set numerical precision = 0.001
Loading file test_geom.h5m
Initializing the GeomQueryTool...
Using faceting tolerance: 0.0001
Building acceleration data structures...
[       OK ] DagSolidTest.test_4 (35 ms)
[ RUN      ] DagSolidTest.test_5
Set overlap thickness = 0
Set numerical precision = 0.001
Loading file test_geom.h5m
Initializing the GeomQueryTool...
Using faceting tolerance: 0.0001
Building acceleration data structures...
[       OK ] DagSolidTest.test_5 (34 ms)
[ RUN      ] DagSolidTest.test_6
Set overlap thickness = 0
Set numerical precision = 0.001
Loading file test_geom.h5m
Initializing the GeomQueryTool...
Using faceting tolerance: 0.0001
Building acceleration data structures...
[       OK ] DagSolidTest.test_6 (34 ms)
[ RUN      ] DagSolidTest.test_7
Set overlap thickness = 0
Set numerical precision = 0.001
Loading file test_geom.h5m
Initializing the GeomQueryTool...
Using faceting tolerance: 0.0001
Building acceleration data structures...
[       OK ] DagSolidTest.test_7 (35 ms)
[ RUN      ] DagSolidTest.test_8
Set overlap thickness = 0
Set numerical precision = 0.001
Loading file test_geom.h5m
Initializing the GeomQueryTool...
Using faceting tolerance: 0.0001
Building acceleration data structures...
[       OK ] DagSolidTest.test_8 (35 ms)
[ RUN      ] DagSolidTest.volume_test
Set overlap thickness = 0
Set numerical precision = 0.001
Loading file test_geom.h5m
Initializing the GeomQueryTool...
Using faceting tolerance: 0.0001
Building acceleration data structures...
[       OK ] DagSolidTest.volume_test (36 ms)
[ RUN      ] DagSolidTest.surface_area_test
Set overlap thickness = 0
Set numerical precision = 0.001
Loading file test_geom.h5m
Initializing the GeomQueryTool...
Using faceting tolerance: 0.0001
Building acceleration data structures...
0
[       OK ] DagSolidTest.surface_area_test (35 ms)
[----------] 16 tests from DagSolidTest (610 ms total)

[----------] Global test environment tear-down
[==========] 16 tests from 1 test case ran. (610 ms total)
[  PASSED  ] 16 tests.

Changelog file

In the CHANGELOG file, I have added a new entry describing the changes made in this PR, including a reference to the pull request number.

helen-brooks commented 1 year ago

This PR is backwards compatible, yes by testing the version of Geant4 with CMake. Given the renewed interest in this I am now dedicating some time to testing with a matrix of geant4 versions so this can be merged in. Then if you require additional changes, those could be merged in on top?

ahnaf-tahmid-chowdhury commented 1 year ago

Thank you for your dedication to testing the compatibility of the PR with multiple versions of Geant4. I'm glad to see that there is renewed interest in this proposal. Please let me know if there is anything I can do to help with the testing or make any necessary changes. I look forward to seeing this PR merged into the main codebase.

helen-brooks commented 1 year ago

This PR is backwards compatible, yes by testing the version of Geant4 with CMake. Given the renewed interest in this I am now dedicating some time to testing with a matrix of geant4 versions so this can be merged in. Then if you require additional changes, those could be merged in on top?

Apologies, I believe I have confused matters. This comment was referring to my PR, #803. This is backwards compatible and works up to the latest in the 10.xx series. My recommendation to the developers would be to merge #803 first, providing there are no additional changes required, then your PR can be merged in on top of that one, to extend compatibility up the 11.xxx series.

My suspicion is that your changes are not backwards compatible as you have not wrapped protected the changes in header files with macros. You might want to take a look at how I did this... But I will leave requests for changes at the discretion on the DAGMC developers.

gonuke commented 1 year ago

I've merged #803 (thanks @helen-brooks ) and recommend that you rebase this PR with those changes and see where we stand.

ahnaf-tahmid-chowdhury commented 1 year ago

I have changed MOAB from version 5.3 to 5.4 and getting the following error

Starting job container
  /usr/bin/docker --config /home/runner/work/_temp/.docker_f73d88b4-b405-4a4c-8e61-b7256efa74dc login ghcr.io -u ahnaf-tahmid-chowdhury --password-stdin
  /usr/bin/docker --config /home/runner/work/_temp/.docker_f73d88b4-b405-4a4c-8e61-b7256efa74dc pull ghcr.io/svalinn/dagmc-ci-ubuntu-18.04-clang-ext-hdf5_1.10.4-moab_5.4.0:stable
  Error response from daemon: manifest unknown
  Warning: Docker pull failed with exit code 1, back off 7.917 seconds before retry.
  /usr/bin/docker --config /home/runner/work/_temp/.docker_f73d88b4-b405-4a4c-8e61-b7256efa74dc pull ghcr.io/svalinn/dagmc-ci-ubuntu-18.04-clang-ext-hdf5_1.10.4-moab_5.4.0:stable
  Error response from daemon: manifest unknown
  Warning: Docker pull failed with exit code 1, back off 2.095 seconds before retry.
  /usr/bin/docker --config /home/runner/work/_temp/.docker_f73d88b4-b405-4a4c-8e61-b7256efa74dc pull ghcr.io/svalinn/dagmc-ci-ubuntu-18.04-clang-ext-hdf5_1.10.4-moab_5.4.0:stable
  Error response from daemon: manifest unknown
  Error: Docker pull failed with exit code 1

The error message indicates that the Docker pull command failed with exit code 1, which means that Docker was unable to download the required image from the specified repository. Any suggestion?

ahnaf-tahmid-chowdhury commented 1 year ago

I am getting the following error while doing Windows Build:

D:\a\DAGMC\install_dir\include\moab/Matrix3.hpp(968): message : see reference to function template instantiation 'bool moab::Util::is_finite<double>(T)' being compiled [D:\a\DAGMC\build\src\uwuw\tests\uwuw_unit_tests_preprocessor.vcxproj]
          with
          [
              T=double
          ]
D:\a\DAGMC\install_dir\include\moab/Util.hpp(65): error C2062: type 'unknown-type' unexpected [D:\a\DAGMC\build\src\uwuw\tests\uwuw_unit_tests_preprocessor.vcxproj]
D:\a\DAGMC\install_dir\include\moab/Util.hpp(65,1): error C2059: syntax error: ')' [D:\a\DAGMC\build\src\uwuw\tests\uwuw_unit_tests_preprocessor.vcxproj]
  uwuw_unit_test_driver.cpp
  Generating Code...
  Building Custom Rule D:/a/DAGMC/DAGMC/src/uwuw/tests/CMakeLists.txt
  uwuw_unit_tests_tally.cpp
D:\a\DAGMC\DAGMC\src\pyne\pyne.h(149,1): warning C4005: 'isnan': macro redefinition [D:\a\DAGMC\build\src\uwuw\tests\uwuw_unit_tests_tally.vcxproj]
D:\a\DAGMC\DAGMC\src\pyne\pyne.h(145): message : see previous definition of 'isnan' [D:\a\DAGMC\build\src\uwuw\tests\uwuw_unit_tests_tally.vcxproj]
D:\a\DAGMC\install_dir\include\moab/Range.hpp(392,14): warning C4251: 'moab::Range::mHead': struct 'moab::Range::PairNode' needs to have dll-interface to be used by clients of class 'moab::Range' [D:\a\DAGMC\build\src\uwuw\tests\uwuw_unit_tests_tally.vcxproj]
D:\a\DAGMC\install_dir\include\moab/Range.hpp(372): message : see declaration of 'moab::Range::PairNode' [D:\a\DAGMC\build\src\uwuw\tests\uwuw_unit_tests_tally.vcxproj]
D:\a\DAGMC\install_dir\include\moab/Range.hpp(944,50): warning C4267: '+=': conversion from 'size_t' to 'unsigned int', possible loss of data [D:\a\DAGMC\build\src\uwuw\tests\uwuw_unit_tests_tally.vcxproj]
D:\a\DAGMC\install_dir\include\moab/Range.hpp(949,39): warning C4267: 'return': conversion from 'size_t' to 'int', possible loss of data [D:\a\DAGMC\build\src\uwuw\tests\uwuw_unit_tests_tally.vcxproj]
D:\a\DAGMC\install_dir\include\moab/Range.hpp(954,35): warning C4267: 'initializing': conversion from 'size_t' to 'unsigned int', possible loss of data [D:\a\DAGMC\build\src\uwuw\tests\uwuw_unit_tests_tally.vcxproj]
D:\a\DAGMC\install_dir\include\moab/Interface.hpp(96,1): warning C4275: non dll-interface class 'moab::UnknownInterface' used as base for dll-interface class 'moab::Interface' [D:\a\DAGMC\build\src\uwuw\tests\uwuw_unit_tests_tally.vcxproj]
D:\a\DAGMC\install_dir\include\moab/UnknownInterface.hpp(96): message : see declaration of 'moab::UnknownInterface' [D:\a\DAGMC\build\src\uwuw\tests\uwuw_unit_tests_tally.vcxproj]
D:\a\DAGMC\install_dir\include\moab/Interface.hpp(95): message : see declaration of 'moab::Interface' [D:\a\DAGMC\build\src\uwuw\tests\uwuw_unit_tests_tally.vcxproj]
D:\a\DAGMC\install_dir\include\moab/Interface.hpp(2056,28): warning C4305: 'return': truncation from 'double' to 'float' [D:\a\DAGMC\build\src\uwuw\tests\uwuw_unit_tests_tally.vcxproj]
  uwuw_unit_test_driver.cpp
  Generating Code...
  uwuw_unit_tests_tally.vcxproj -> D:\a\DAGMC\build\src\uwuw\tests\Release\uwuw_unit_tests_tally.exe
Error: Process completed with exit code 1.
##[debug]Finishing: build DAGMC
ahnaf-tahmid-chowdhury commented 1 year ago

Dear @gonuke, I think it is time to run our tests as the windows build issue is solved. After that, I may replace Gent4 version from 10.5.1 to 11.1.1 in build_geant4.sh and run the tests again.

ahnaf-tahmid-chowdhury commented 1 year ago 
  uwuw_unit_test_driver.cpp
  Generating Code...
  uwuw_unit_tests.vcxproj -> D:\a\DAGMC\build\src\uwuw\tests\Release\uwuw_unit_tests.exe
  Building Custom Rule D:/a/DAGMC/DAGMC/src/uwuw/tests/CMakeLists.txt
  uwuw_unit_tests_preprocessor.cpp
D:\a\DAGMC\DAGMC\src\pyne\pyne.h(149,1): warning C4005: 'isnan': macro redefinition [D:\a\DAGMC\build\src\uwuw\tests\uwuw_unit_tests_preprocessor.vcxproj]
D:\a\DAGMC\DAGMC\src\pyne\pyne.h(145): message : see previous definition of 'isnan' [D:\a\DAGMC\build\src\uwuw\tests\uwuw_unit_tests_preprocessor.vcxproj]
D:\a\DAGMC\install_dir\include\moab/Range.hpp(392,14): warning C4251: 'moab::Range::mHead': struct 'moab::Range::PairNode' needs to have dll-interface to be used by clients of class 'moab::Range' [D:\a\DAGMC\build\src\uwuw\tests\uwuw_unit_tests_preprocessor.vcxproj]
D:\a\DAGMC\install_dir\include\moab/Range.hpp(372): message : see declaration of 'moab::Range::PairNode' [D:\a\DAGMC\build\src\uwuw\tests\uwuw_unit_tests_preprocessor.vcxproj]
D:\a\DAGMC\install_dir\include\moab/Range.hpp(944,50): warning C4267: '+=': conversion from 'size_t' to 'unsigned int', possible loss of data [D:\a\DAGMC\build\src\uwuw\tests\uwuw_unit_tests_preprocessor.vcxproj]
D:\a\DAGMC\install_dir\include\moab/Range.hpp(949,39): warning C4267: 'return': conversion from 'size_t' to 'int', possible loss of data [D:\a\DAGMC\build\src\uwuw\tests\uwuw_unit_tests_preprocessor.vcxproj]
D:\a\DAGMC\install_dir\include\moab/Range.hpp(954,35): warning C4267: 'initializing': conversion from 'size_t' to 'unsigned int', possible loss of data [D:\a\DAGMC\build\src\uwuw\tests\uwuw_unit_tests_preprocessor.vcxproj]
D:\a\DAGMC\install_dir\include\moab/Interface.hpp(96,1): warning C4275: non dll-interface class 'moab::UnknownInterface' used as base for dll-interface class 'moab::Interface' [D:\a\DAGMC\build\src\uwuw\tests\uwuw_unit_tests_preprocessor.vcxproj]
D:\a\DAGMC\install_dir\include\moab/UnknownInterface.hpp(96): message : see declaration of 'moab::UnknownInterface' [D:\a\DAGMC\build\src\uwuw\tests\uwuw_unit_tests_preprocessor.vcxproj]
D:\a\DAGMC\install_dir\include\moab/Interface.hpp(95): message : see declaration of 'moab::Interface' [D:\a\DAGMC\build\src\uwuw\tests\uwuw_unit_tests_preprocessor.vcxproj]
D:\a\DAGMC\install_dir\include\moab/Interface.hpp(2056,28): warning C4305: 'return': truncation from 'double' to 'float' [D:\a\DAGMC\build\src\uwuw\tests\uwuw_unit_tests_preprocessor.vcxproj]
D:\a\DAGMC\install_dir\include\moab/FileOptions.hpp(240,32): warning C4251: 'moab::FileOptions::mOptions': class 'std::vector<const char *,std::allocator<const char *>>' needs to have dll-interface to be used by clients of class 'moab::FileOptions' [D:\a\DAGMC\build\src\uwuw\tests\uwuw_unit_tests_preprocessor.vcxproj]
D:\a\DAGMC\install_dir\include\moab/FileOptions.hpp(240): message : see declaration of 'std::vector<const char *,std::allocator<const char *>>' [D:\a\DAGMC\build\src\uwuw\tests\uwuw_unit_tests_preprocessor.vcxproj]
D:\a\DAGMC\install_dir\include\moab/FileOptions.hpp(241,33): warning C4251: 'moab::FileOptions::mSeen': class 'std::vector<bool,std::allocator<bool>>' needs to have dll-interface to be used by clients of class 'moab::FileOptions' [D:\a\DAGMC\build\src\uwuw\tests\uwuw_unit_tests_preprocessor.vcxproj]
D:\a\DAGMC\install_dir\include\moab/FileOptions.hpp(241): message : see declaration of 'std::vector<bool,std::allocator<bool>>' [D:\a\DAGMC\build\src\uwuw\tests\uwuw_unit_tests_preprocessor.vcxproj]
D:\a\DAGMC\install_dir\include\moab/FileOptions.hpp(208,1): warning C4267: 'return': conversion from 'size_t' to 'unsigned int', possible loss of data [D:\a\DAGMC\build\src\uwuw\tests\uwuw_unit_tests_preprocessor.vcxproj]
D:\a\DAGMC\install_dir\include\moab/GeomTopoTool.hpp(355,34): warning C4267: 'return': conversion from 'size_t' to 'int', possible loss of data [D:\a\DAGMC\build\src\uwuw\tests\uwuw_unit_tests_preprocessor.vcxproj]
D:\a\DAGMC\install_dir\include\moab/GeomTopoTool.hpp(363,53): warning C4267: 'initializing': conversion from 'size_t' to 'unsigned int', possible loss of data [D:\a\DAGMC\build\src\uwuw\tests\uwuw_unit_tests_preprocessor.vcxproj]
  WARNING: You need to implement DEPRECATED for this compiler
D:\a\DAGMC\install_dir\include\moab/GeomQueryTool.hpp(95,1): warning C4267: 'return': conversion from 'size_t' to 'int', possible loss of data [D:\a\DAGMC\build\src\uwuw\tests\uwuw_unit_tests_preprocessor.vcxproj]
D:\a\DAGMC\DAGMC\src\dagmc\DagMC.hpp(574,42): warning C4267: 'return': conversion from 'size_t' to 'unsigned int', possible loss of data [D:\a\DAGMC\build\src\uwuw\tests\uwuw_unit_tests_preprocessor.vcxproj]
D:\a\DAGMC\install_dir\include\moab/Util.hpp(65,12): error C2589: '(': illegal token on right side of '::' [D:\a\DAGMC\build\src\uwuw\tests\uwuw_unit_tests_preprocessor.vcxproj]
D:\a\DAGMC\install_dir\include\moab/Matrix3.hpp(968): message : see reference to function template instantiation 'bool moab::Util::is_finite<double>(T)' being compiled [D:\a\DAGMC\build\src\uwuw\tests\uwuw_unit_tests_preprocessor.vcxproj]
          with
          [
              T=double
          ]
D:\a\DAGMC\install_dir\include\moab/Util.hpp(65): error C2062: type 'unknown-type' unexpected [D:\a\DAGMC\build\src\uwuw\tests\uwuw_unit_tests_preprocessor.vcxproj]
D:\a\DAGMC\install_dir\include\moab/Util.hpp(65,1): error C2059: syntax error: ')' [D:\a\DAGMC\build\src\uwuw\tests\uwuw_unit_tests_preprocessor.vcxproj]
  uwuw_unit_test_driver.cpp
ahnaf-tahmid-chowdhury commented 1 year ago

Ok. There is a syntax issue with MOAB v5.4.1. With v5.3 its passing the windows build.

makeclean commented 1 year ago

Do you know the delta change between MOAB 5.3.0 and MOAB 5.4.1 for Matrix3.hpp?

ahnaf-tahmid-chowdhury commented 1 year ago

Dear @makeclean, I apologize for my limited knowledge of MOAB, but based on the release notes, MOAB 5.4 introduced an improved iMOAB API for Fortran compatibility, while MOAB 5.4.1 added a new routine, iMOAB_SetDoubleTagStorageWithGid, to store values in a MOAB double Tag in iMOAB.

makeclean commented 1 year ago

Ah, ok this issue with MOAB is this line, (RHS is MOAB 5.3.1)

image

So it looks like the VSC++ compiler doesn't like the std::isfinite, maybe theres and additonal flag we can pass it?

ahnaf-tahmid-chowdhury commented 1 year ago

Ah, ok this issue with MOAB is this line, (RHS is MOAB 5.3.1)

image

So it looks like the VSC++ compiler doesn't like the std::isfinite, maybe theres and additonal flag we can pass it?

I have added additional compiler flag for Visual Studio std:c++17. Let's see

However, moab_isfinite macro is defined based on whether certain macros (MOAB_HAVE_ISFINITE, MOAB_HAVE_STDISFINITE and MOAB_HAVE_FINITE) are defined or not. I think we need to ensure that one of the macros is defined in our system.

ahnaf-tahmid-chowdhury commented 1 year ago

I am going to make a new PR for this new MOAB. For now, I think we may fix the Geant4 with MOAB 5.3.

ahnaf-tahmid-chowdhury commented 1 year ago

Without running clang-format, housekeeping is successful. But after running clang-format, it fails. Log

ahnaf-tahmid-chowdhury commented 1 year ago

We've successfully passed v10.5 and I believe it's about time we consider upgrading to newer versions. @gonuke, please share your thoughts on this.

gonuke commented 1 year ago

We're definitely getting closer! Unfortunately, our testing is still not fully converted to the new system until I resolve #880. Then it should be straightforward to test this update with newer versions of Geant

gonuke commented 11 months ago

I think all of our workflows are in place to test this properly now. You may need to rebase and then launch some tests manually on your fork, and/or add Geant 11.x to the workflow matrix?

gonuke commented 11 months ago

I’m a little confused by the current state of this PR. There are many files that are included here that are already changed in the current develop branch.

I think the best approach is to rebase this branch onto svalinn/develop so that it is more clear exactly which changes are from your addition here.

ahnaf-tahmid-chowdhury commented 11 months ago

I accidentally forked @shimwell's repo, and it is not synced with the upstream svalinn/develop branch. Should I close this PR and create a new one? Or, if @shimwell kindly updates his develop branch, then I can rebase this branch.

gonuke commented 11 months ago

I accidentally forked @shimwell's repo, and it is not synced with the upstream svalinn/develop branch. Should I close this PR and create a new one? Or, if @shimwell kindly updates his develop branch, then I can rebase this

I think it's too late now because you force pushed over your develop branch, but for future reference, you can rebase from any branch on any repo on the command-line/client-side.