Updated DAGMC code to be compatible with Geant4 v11.1.1 #860
Conversation
|
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? |
|
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. |
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. |
|
I've merged #803 (thanks @helen-brooks ) and recommend that you rebase this PR with those changes and see where we stand. |
|
I have changed MOAB from version 5.3 to 5.4 and getting the following error 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. |
There was a problem hiding this comment.
Thanks for this update @ahnaf-tahmid-chowdhury. I think you've introduced a big question to our team about which/how many versions of Geant4 we should actively support and regularly test! It's a heavy dependency and resource intensive to support multiple versions.
Do you have any perspective on which versions are most common?
CI/docker/build_geant4.sh
Outdated
| @@ -3,10 +3,10 @@ | |||
| set -ex | |||
|
|
|||
| # Geant version and corresponding SHASUM | |||
| export geant4_version=10.5.1 | |||
| export geant4_version=11.1.1 | |||
There was a problem hiding this comment.
It will useful to test these changes with v10.5 first and then upgrade to other versions. We may also want a strategy for which versions we'll support over time.
There was a problem hiding this comment.
I have set the version to 10.5.1 again. Regarding the strategy for the supported versions, we can discuss and decide on that together to ensure a smooth and reliable development process.
There was a problem hiding this comment.
According to the Geant4 website, the latest stable version as of my knowledge cutoff (Fab 2023) was Geant4 version 11.1.1, and it's recommended for new users to start with the latest version. Additionally, they are also supporting version 10.7 and the latest version is 10.7.4 which was released in Sep 2022.
Based on the latest Geant4 release notes, version 11 introduces significant updates and improvements that could benefit the performance and accuracy of simulations using DAGMC. Therefore, it may be advantageous for users to consider upgrading to the latest version of Geant4 when working with the latest release of DAGMC.
CI/docker/build_geant4.sh
Outdated
| @@ -3,10 +3,10 @@ | |||
| set -ex | |||
|
|
|||
| # Geant version and corresponding SHASUM | |||
| export geant4_version=10.5.1 | |||
| export geant4_version=11.1.1 | |||
There was a problem hiding this comment.
I have set the version to 10.5.1 again. Regarding the strategy for the supported versions, we can discuss and decide on that together to ensure a smooth and reliable development process.
|
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 |
|
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 |
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 |
|
Ok. There is a syntax issue with MOAB v5.4.1. With v5.3 its passing the windows build. |
|
Do you know the delta change between MOAB 5.3.0 and MOAB 5.4.1 for Matrix3.hpp? |
|
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, |
|
Ah, ok this issue with MOAB is this line, (RHS is MOAB 5.3.1)
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, |
|
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. |
|
Without running |
|
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. |
|
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 |
There was a problem hiding this comment.
I've noted a few things here, but we'll also want to consider how we start adding different geant versions to our build environment.
I think if you rebase this branch on the recent additions to develop you can include something in the github workflows to make other Geant versions available.
cmake/FindGeant4.cmake
Outdated
| find_path(Geant4_CMAKE_CONFIG_VERSION | ||
| NAMES Geant4ConfigVersion.cmake | ||
| PATHS ${Geant4_SEARCH_DIRS} | ||
| NO_DEFAULT_PATH | ||
| ) | ||
| # Extract the version number from the first directory in Geant4_SEARCH_DIRS | ||
| if(Geant4_SEARCH_DIRS) | ||
| # Extract the version number from the directory name | ||
| list(GET Geant4_SEARCH_DIRS 0 Geant4_LIB_DIR) | ||
| file(RELATIVE_PATH Geant4_VERSION ${GEANT4_DIR} ${Geant4_LIB_DIR}) | ||
| string(REGEX MATCH "[0-9]+\\.[0-9]+\\.[0-9]+" Geant4_VERSION ${Geant4_VERSION}) | ||
|
|
||
| if (Geant4_CMAKE_CONFIG_VERSION) | ||
| set(Geant4_CMAKE_CONFIG_VERSION ${Geant4_CMAKE_CONFIG_VERSION}/Geant4ConfigVersion.cmake) |
There was a problem hiding this comment.
Does Geant no longer provide a cmake file that we can interrogate? That seems preferable to parsing a string based on a directory name....
There was a problem hiding this comment.
I'm unsure whether Geant4 provides and maintains this file within its distribution. I have developed a method to extract the version of the directory structure itself. The advantage is that it can work even if Geant4 doesn't provide a version-specific configuration file. It directly extracts version information from the installation path. However, the downside is that it's vulnerable to changes in the directory structure, which could potentially lead to parsing errors. It relies on consistent naming conventions.
There was a problem hiding this comment.
One of the values of CMake is that is automates this in a robust way. I'll take a look to see what I find in a new distribution
There was a problem hiding this comment.
Yes. Geant4 provides and maintains this file within its distribution.
I am testing DAGMC with following arguments and its working. Let's see if it passes the workflow.
cmake ../ -DMOAB_CMAKE_CONFIG=$env_dir/lib/cmake/MOAB \
-DMOAB_DIR=$env_dir \
-DBUILD_STATIC_LIBS=OFF \
-DBUILD_GEANT4=ON \
-DGeant4_CMAKE_CONFIG=$env_dir/lib/cmake/Geant4 \
-DGEANT4_DIR=$env_dir \
-DBUILD_TALLY=ON \
-DCMAKE_INSTALL_PREFIX=$env_dir
src/geant4/generate_geant4
Outdated
| @@ -2,9 +2,11 @@ | |||
|
|
|||
| from optparse import OptionParser | |||
| import os,errno | |||
| from itaps import iMesh,iBase | |||
| import sys | |||
| import meshio | |||
There was a problem hiding this comment.
I think we should convert the itaps code to use the pyne.dagmc class rather than the meshio module
There was a problem hiding this comment.
I am unable to run the tests as I am unfamiliar how to run this code.
It seams the code take some arguments and need some geometry_file.h5m. Can you please provide me the tutorial link?
and are you asking to use pyne.dagmc.load?
|
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? |
|
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 I think the best approach is to rebase this branch onto |
I think it's too late now because you force pushed over your |
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.ccandgenerate_geant4which 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.
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.