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.

Sign up for GitHub

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

Automatic download and build of MOAB #969

Open
wants to merge 22 commits into
base: develop
Choose a base branch
from
+156 −21
Open

Automatic download and build of MOAB #969

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
861807f
first attempt to auto-install MOAB
bam241 Dec 3, 2024
a5e83e3
Update cmake/FindMOAB.cmake
bam241 Dec 5, 2024
b28d951
Update cmake/FindMOAB.cmake
bam241 Dec 5, 2024
26e03af
reformating + changelog + tests
bam241 Dec 6, 2024
0908fdd
add missing indent
bam241 Dec 6, 2024
83ca15e
generalyze shared lib extention
bam241 Dec 6, 2024
c6df67a
temporary change to master
bam241 Dec 6, 2024
853b3b9
back to 5.5.1
bam241 Dec 6, 2024
612893a
adding DDL_INSTALL_DEPS as an option with default to OFF
bam241 Dec 12, 2024
b012285
keeping jon changes
bam241 Dec 12, 2024
bbe4828
trying MOAB master
bam241 Dec 12, 2024
ef5a2e0
reverting to 5.5.1 as default version for MOAB
bam241 Dec 12, 2024
6675cef
cleaning variable name
bam241 Dec 12, 2024
19da17b
moving this up one level
bam241 Jan 10, 2025
bf4ecff
missing a )
bam241 Jan 10, 2025
ef2c3e5
cmake likes set to define variables
bam241 Jan 10, 2025
e7a03fd
no need for MOAB_VERSION
bam241 Jan 10, 2025
5907867
how about now ?
bam241 Jan 10, 2025
8a8597d
how about now ?
bam241 Jan 10, 2025
9d4ccc0
no caps for include folder
bam241 Jan 10, 2025
445aab9
revamping find_MOAB
bam241 Jan 10, 2025
b1232be
adressing @gonuke comments
bam241 Jan 13, 2025
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
12 changes: 12 additions & 0 deletions .github/workflows/linux_build_test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ jobs:
5.4.1,
5.5.1,
]
pull_install_moab : [
off,
]
geant4_version : [
10.7.4,
11.1.2
Expand All @@ -58,6 +61,14 @@ jobs:
off,
v1.1.0,
]
include:
- ubuntu_version: 22.04
compiler: gcc
hdf5_version: 1.14.3
moab_version: 5.4.1
pull_install_moab: on
geant4_version: off
double_down_version: off

container:
image: ghcr.io/svalinn/dagmc-ci-ubuntu-${{
Expand Down Expand Up @@ -88,6 +99,7 @@ jobs:
cmake ../ \
-DMOAB_DIR=${moab_install_dir} \
-DBUILD_GEANT4=$([ "${{ matrix.geant4_version }}" != "off" ] && echo "ON" || echo "OFF") \
-DPULL_INSTALL_MOAB=$([ "${{ matrix.pull_install_moab }}" != "on" ] && echo "OFF" || echo "ON -DHDF5_ROOT=${hdf5_install_dir}") \
-DGEANT4_DIR=${geant4_install_dir} \
-DBUILD_CI_TESTS=ON \
-DBUILD_MW_REG_TESTS=OFF \
Expand Down
2 changes: 1 addition & 1 deletion CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
project(DAGMC)
cmake_minimum_required(VERSION 3.18)
cmake_minimum_required(VERSION 3.1)
bam241 marked this conversation as resolved.
Show resolved Hide resolved
enable_language(CXX)

# Set DAGMC version
Expand Down
7 changes: 7 additions & 0 deletions cmake/DAGMC_macros.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ macro (dagmc_setup_options)

option(DOUBLE_DOWN "Enable ray tracing with Embree via double down" OFF)

option(PULL_INSTALL_MOAB "Enable automatic downloading of MOAB dependency" OFF)

if (BUILD_ALL)
set(BUILD_MCNP5 ON)
set(BUILD_MCNP6 ON)
Expand Down Expand Up @@ -253,6 +255,11 @@ macro (dagmc_install_library lib_name)
EXPORT DAGMCTargets
LIBRARY DESTINATION ${INSTALL_LIB_DIR}
PUBLIC_HEADER DESTINATION ${INSTALL_INCLUDE_DIR})
# Required to ensure that MOAB is built before DAGMC and to properly link against MOAB
if(PULL_INSTALL_MOAB)
add_dependencies(${lib_name}-shared MOAB)
target_link_libraries(${lib_name}-shared PUBLIC ${MOAB_LIBRARY_DIRS}/libMOAB${CMAKE_SHARED_LIBRARY_SUFFIX})
endif()
endif ()

if (BUILD_STATIC_LIBS)
Expand Down
21 changes: 18 additions & 3 deletions cmake/FindMOAB.cmake
View file
Open in desktop
bam241 marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,28 @@ find_path(MOAB_CMAKE_CONFIG
PATHS ${MOAB_SEARCH_DIRS}
NO_DEFAULT_PATH
)
if (MOAB_CMAKE_CONFIG)

# First check if we are forcing the download of MOAB
if (PULL_INSTALL_MOAB)
Copy link
Member

Choose a reason for hiding this comment

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

Have we thought about whether we put this condition up one level? That is, if we are going to PULL_INSTALL, then we don't call FindPackage at all and just do this macro instead? Then we don't need to mess with the FindMOAB file. I think that will work???

if(DAGMC_BUILD_STATIC_LIBS)
message(FATAL_ERROR "PULL_INSTALL_MOAB is ONLY compatible with shared libraries.")
endif()
if(NOT MOAB_VERSION)
set(MOAB_VERSION "5.5.1")
endif()
include(MOAB_PullAndMake)
moab_pull_make(${MOAB_VERSION})

# Back to normal behavior
elseif (MOAB_CMAKE_CONFIG)
set(MOAB_CMAKE_CONFIG ${MOAB_CMAKE_CONFIG}/MOABConfig.cmake)
include(${MOAB_CMAKE_CONFIG})
message(STATUS "MOAB_CMAKE_CONFIG: ${MOAB_CMAKE_CONFIG}")
else ()
message(FATAL_ERROR "Could not find MOAB. Set -DMOAB_DIR=<MOAB_DIR> when running cmake or use the $MOAB_DIR environment variable.")
endif ()

# Find HDF5
include(${MOAB_CMAKE_CONFIG})
set(ENV{PATH} "${HDF5_DIR}:$ENV{PATH}")
set(CMAKE_FIND_LIBRARY_SUFFIXES ${CMAKE_SHARED_LIBRARY_SUFFIX})
find_package(HDF5 REQUIRED)
Expand Down Expand Up @@ -71,7 +84,9 @@ message(STATUS "MOAB_LIBRARY_DIRS: ${MOAB_LIBRARY_DIRS}")
message(STATUS "MOAB_LIBRARIES_SHARED: ${MOAB_LIBRARIES_SHARED}")
message(STATUS "MOAB_LIBRARIES_STATIC: ${MOAB_LIBRARIES_STATIC}")

if (MOAB_INCLUDE_DIRS AND (MOAB_LIBRARIES_SHARED OR NOT BUILD_SHARED_LIBS) AND
if(PULL_INSTALL_MOAB)
message(STATUS "MOAB will be downloaded and built at make time")
elseif (MOAB_INCLUDE_DIRS AND (MOAB_LIBRARIES_SHARED OR NOT BUILD_SHARED_LIBS) AND
(MOAB_LIBRARIES_STATIC OR NOT BUILD_STATIC_LIBS))
message(STATUS "Found MOAB")
else ()
Expand Down
45 changes: 45 additions & 0 deletions cmake/MOAB_PullAndMake.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
# this Macro sets up the download and build of MOAB using ExternalProject
# few tweak are done in src/dagmc/CMakeLists.txt and src/PyNE/CMakelists.txt
# to make sure that MOAB is built before DAGMC.
MACRO (moab_pull_make moab_version)
message(STATUS "MOAB will be downloaded and built")
include(ExternalProject)
message("HDF5_ROOT: ${HDF5_ROOT}")
set(MOAB_INSTALL_PREFIX "${CMAKE_INSTALL_PREFIX}/moab")
set(MOAB_ROOT "${CMAKE_BINARY_DIR}/moab")
set(MOAB_INCLUDE_DIRS "${MOAB_INSTALL_PREFIX}/include")
set(MOAB_LIBRARY_DIRS "${MOAB_INSTALL_PREFIX}/lib")
message("MOAB_LIBRARY_DIRS: ${MOAB_LIBRARY_DIRS}")
message("CMAKE_SHARED_LIBRARY_SUFFIX: ${CMAKE_SHARED_LIBRARY_SUFFIX}")
set(MOAB_LIBRARIES_SHARED "")
ExternalProject_Add(MOAB_ep
PREFIX ${MOAB_ROOT}
GIT_REPOSITORY https://bitbucket.org/fathomteam/moab.git
GIT_TAG ${moab_version}
CMAKE_ARGS
-DCMAKE_INSTALL_PREFIX:PATH=<INSTALL_DIR>
-DBUILD_SHARED_LIBS:BOOL=ON
-DENABLE_HDF5:BOOL=ON
-DHDF5_ROOT:PATH=${HDF5_ROOT}
-DCMAKE_INSTALL_RPATH=${HDF5_ROOT}/lib:${MOAB_INSTALL_PREFIX}/lib
Copy link
Member

Choose a reason for hiding this comment

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

This method may work here for now, but may create trouble while working with scikit-build-core depended project. However, I am not sure here.

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 have no experience about scikit-build-core, maybe we shall fix it, if the problem occurs ?

Copy link
Member

Choose a reason for hiding this comment

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

I believe MOAB master branch now has scikit-build core while version 5.5.1 does not.

Perhaps changing the moab version to master would provide a quick way of checking if this continues to work

      GIT_REPOSITORY https://bitbucket.org/fathomteam/moab.git
      GIT_TAG ${moab_version}

Copy link
Member Author

Choose a reason for hiding this comment

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

Trying it now.

Copy link

Choose a reason for hiding this comment

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

Just to mention @bam241 tried this with the master branch of MOAB (which has scikit build core) and reported that it works

Copy link
Member Author

Choose a reason for hiding this comment

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

have a doubt about what I did, I'll try one more time

Copy link
Member

Choose a reason for hiding this comment

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

Any final resolution on this?

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 have tested this against MOAB master, this works.

Test on 5fe643b
checked the proper work of this against MOAB Master branch

-DENABLE_BLASLAPACK:BOOL=OFF
-DENABLE_FORTRAN:BOOL=OFF
-DENABLE_PYMOAB:BOOL=OFF
DOWNLOAD_EXTRACT_TIMESTAMP true
BUILD_BYPRODUCTS "${MOAB_LIBRARY_DIRS}/*${CMAKE_SHARED_LIBRARY_SUFFIX}*"
INSTALL_DIR "${MOAB_INSTALL_PREFIX}"
)
# Setup a interface library for MOAB based on ExternalProoject MOAB_EP
add_library(MOAB INTERFACE)

target_include_directories(MOAB SYSTEM INTERFACE ${MOAB_INCLUDE_DIRS})
target_link_libraries(MOAB INTERFACE ${MOAB_LIBRARY_DIRS}/libMOAB${CMAKE_SHARED_LIBRARY_SUFFIX})
add_dependencies(MOAB MOAB_ep)
install(TARGETS MOAB LIBRARY DESTINATION ${MOAB_LIBRARY_DIRS}
PUBLIC_HEADER DESTINATION ${INSTALL_INCLUDE_DIR})
include_directories(${MOAB_INCLUDE_DIRS})
link_directories(${MOAB_LIBRARY_DIRS})
find_package(Eigen3 REQUIRED NO_MODULE)
include_directories(${EIGEN3_INCLUDE_DIRS})

ENDMACRO(moab_pull_make)
3 changes: 3 additions & 0 deletions doc/CHANGELOG.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ DAGMC Changelog
Next version
====================

**Added:**
* Allow download from cmake and compilation at build time of MOAB (#969)

v3.2.4
====================

Expand Down
5 changes: 5 additions & 0 deletions src/mcnp/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ add_library(mcnp_funcs OBJECT mcnp_funcs.cpp)
message(STATUS "Building object library: meshtal_funcs")
add_library(meshtal_funcs OBJECT meshtal_funcs.cpp)

if(PULL_INSTALL_MOAB)
Copy link
Member

Choose a reason for hiding this comment

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

I'm curious as to why these lines are needed now when they weren't before.

Copy link
Member Author

Choose a reason for hiding this comment

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

The goal is to allow if asked for, the possibility to pull and compile MOAB at the DAGMC build time

This variable has been added to detect if such auto download and compilation of MOAB is requested.

Copy link
Member

Choose a reason for hiding this comment

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

I think @pshriwise has a different question: why does this method explicitly require adding the dependencies? As I recall from a conversation last month, since the MOAB libraries are not built until build time, CMake can't add them implicitly. They are not found by CMake and added to the linked library list, so we have to tell it to happend.

add_dependencies(mcnp_funcs MOAB)
add_dependencies(meshtal_funcs MOAB)
endif()

if (BUILD_MCNP5)
add_subdirectory(mcnp5)
endif ()
Expand Down
Loading