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

Allow Double Down v1.1.0 Installation in Dockerfile #944

Merged
merged 7 commits into from
Feb 29, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/actions/upstream-test/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ inputs:
moab_version:
description: Version of MOAB
required: false
default: 5.3.0
default: 5.5.1
double_down_version:
description: Version of Double Down
required: false
Expand Down
2 changes: 2 additions & 0 deletions .github/workflows/docker_publish.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ jobs:
5.5.1,
]
geant4_version : [
off,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure it's worth have an extra image without GEANT?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While updating action.yml, I have noticed we are not using Geant4 by default. Do you like to change the default?

Copy link
Member

@gonuke gonuke Feb 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The treatment of GEANT and Double Down are a little different from each other.

  1. Testing with or without GEANT only demonstrates whether DAGMC will integrate successfully with GEANT as a transport code.

  2. Testing with or without Double Down modifies the underlying fundamental behavior of DAGMC with all transport codes.

A few conclusions of this:

A) It makes sense to have fewer images, i.e. none that are build without GEANT and without DD because we can test our build without them even if they are installed.

B) It makes sense to do the advisory testing with the minimal number of options turned on, ie. without GEANT even if it is installed in the image.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've made changes to streamline the build process and reduce the number of arguments as much as possible. But it seems, I've ended up creating extra Docker images. 🙂

I think we have two options:

  1. Add the extra arguments again and remove conditions from the Dockerfile.
  2. Consider setting GEANT4 and DD always on in action.yml.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me know, which step to proceed. Or, any other suggestions?

10.7.4,
11.1.2
]
Expand Down Expand Up @@ -111,6 +112,7 @@ jobs:
5.5.1,
]
geant4_version : [
off,
10.7.4,
11.1.2
]
Expand Down
2 changes: 1 addition & 1 deletion cmake/DAGMC_macros.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ macro (dagmc_setup_options)

if (DOUBLE_DOWN)
find_package(DOUBLE_DOWN REQUIRED)
if (DOUBLE_DOWN_VERSION VERSION_LESS 1.1.0)
if (dd_VERSION VERSION_LESS 1.1.0)
message(FATAL_ERROR "Discovered Double Down Version: ${DOUBLE_DOWN_VERSION}. \
Please update Double Down to version 1.1.0 or greater.")
endif()
Expand Down
13 changes: 10 additions & 3 deletions cmake/FindDOUBLE_DOWN.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
# dd_INCLUDE_DIRS - include directories for installed dd headers
# dd_LIBRARY_DIRS - location of installed dd libraries
# dd_LIBRARIES - set of libraries installed with dd, use to link applications against dd
# dd_VERSION - version of installed dd

find_path(dd_CMAKE_CONFIG NAMES ddConfig.cmake
HINTS ${dd_ROOT} $ENV{dd_ROOT}
Expand All @@ -14,6 +15,12 @@ find_path(dd_CMAKE_CONFIG NAMES ddConfig.cmake
PATH_SUFFIXES lib Lib cmake lib/cmake/ lib/cmake/dd
NO_DEFAULT_PATH)

message(STATUS "Found dd in ${dd_CMAKE_CONFIG}")

include(${dd_CMAKE_CONFIG}/ddConfig.cmake)
if (dd_CMAKE_CONFIG)
message(STATUS "Found dd in ${dd_CMAKE_CONFIG}")
include(${dd_CMAKE_CONFIG}/ddConfig.cmake)
include(${dd_CMAKE_CONFIG}/ddConfigVersion.cmake)
set(dd_VERSION ${PACKAGE_VERSION})
message(STATUS "Found dd version ${dd_VERSION}")
else()
message(FATAL_ERROR "Could not find dd")
endif()
2 changes: 1 addition & 1 deletion doc/CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ Next version
* Update MOAB to 5.5.1 from 5.3.0 (#939 #940)
* Update README regarding OpenMC (#938)
* Simplify Housekeeping Process for DAGMC (#943)
* Allow Double Down v1.1.0 Installation in Dockerfile (#929)
* Allow Double Down v1.1.0 Installation in Dockerfile (#929 #944)

v3.2.3
====================
Expand Down
Loading