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

Build NCrystal from Source for OpenMC #3304

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from
Open

Build NCrystal from Source for OpenMC #3304

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
233064b
source build NCrystal
ahnaf-tahmid-chowdhury Feb 14, 2025
b8556c6
fix variables
ahnaf-tahmid-chowdhury Feb 14, 2025
72f653a
chmod +x
ahnaf-tahmid-chowdhury Feb 14, 2025
8eb71c2
chmod +x ./tools/ci/gha-install-ncrystal.sh
ahnaf-tahmid-chowdhury Feb 14, 2025
ac7b4f2
skip nctool
ahnaf-tahmid-chowdhury Feb 14, 2025
2be024a
fix tests
ahnaf-tahmid-chowdhury Feb 14, 2025
9e900f2
install only the python module of NCrystal
ahnaf-tahmid-chowdhury Feb 14, 2025
254c4c8
pip install ncrystal
ahnaf-tahmid-chowdhury Feb 17, 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: 2 additions & 10 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,16 +121,8 @@ endmacro()
#===============================================================================

if(OPENMC_USE_NCRYSTAL)
if(NOT DEFINED "NCrystal_DIR")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have removed this as it forces the definition of NCrystal_DIR, which would break the standard method of defining other CMake variables for NCrystal. However, if this is recommended, please let me know.

#Invocation of "ncrystal-config --show cmakedir" is needed to find NCrystal
#when it is installed from Python wheels:
execute_process(
COMMAND "ncrystal-config" "--show" "cmakedir"
OUTPUT_VARIABLE "NCrystal_DIR" OUTPUT_STRIP_TRAILING_WHITESPACE
)
Comment on lines -127 to -130
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we can make it available with a different fallback, right?

endif()
find_package(NCrystal 3.8.0 REQUIRED)
message(STATUS "Found NCrystal: ${NCrystal_DIR} (version ${NCrystal_VERSION})")
find_package(NCrystal 4.0.0 REQUIRED)
message(STATUS "Found NCrystal: ${NCrystal_LIBRARIES} (version ${NCrystal_VERSION})")
endif()

#===============================================================================
Expand Down
11 changes: 1 addition & 10 deletions cmake/OpenMCConfig.cmake.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,7 @@ if(@OPENMC_USE_DAGMC@)
endif()

if(@OPENMC_USE_NCRYSTAL@)
if(NOT DEFINED "NCrystal_DIR")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is broken. We should set NCrystal_DIR to @NCrystal_DIR@ at the top of OpenMC's config file. The current version can break OpenMC, as the installed version of NCrystal might be different. For example, OpenMC could have been built with NCrystal 4.0.0, while the latest version of NCrystal from PyPI might be 4.1.0.

#Invocation of "ncrystal-config --show cmakedir" is needed to find NCrystal
#when it is installed from Python wheels:
execute_process(
COMMAND "ncrystal-config" "--show" "cmakedir"
OUTPUT_VARIABLE "NCrystal_DIR" OUTPUT_STRIP_TRAILING_WHITESPACE
)
endif()
find_package(NCrystal REQUIRED)
message(STATUS "Found NCrystal: ${NCrystal_DIR} (version ${NCrystal_VERSION})")
find_package(NCrystal REQUIRED HINTS @CMAKE_PREFIX_PATH@)
endif()

if(@OPENMC_USE_LIBMESH@)
Expand Down
27 changes: 27 additions & 0 deletions tools/ci/gha-install-ncrystal.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
#!/bin/bash
set -ex

# NCrystal Variables
NCRYSTAL_BRANCH='v4.0.0'
NCRYSTAL_REPO='https://github.com/mctools/ncrystal'
NCRYSTAL_INSTALL_DIR=$HOME/NCRYSTAL/

CURRENT_DIR=$(pwd)

# NCrystal Install
cd $HOME
git clone -b $NCRYSTAL_BRANCH $NCRYSTAL_REPO ncrystal
cd ncrystal
mkdir build && cd build
cmake .. \
-DBUILD_SHARED_LIBS=ON \
-DNCRYSTAL_NOTOUCH_CMAKE_BUILD_TYPE=ON \
-DNCRYSTAL_MODIFY_RPATH=OFF \
-DCMAKE_BUILD_TYPE=Release \
-DNCRYSTAL_ENABLE_EXAMPLES=OFF \
-DNCRYSTAL_ENABLE_SETUPSH=OFF \
-DNCRYSTAL_ENABLE_DATA=EMBED \
-DCMAKE_INSTALL_PREFIX="$NCRYSTAL_INSTALL_DIR"
make -j4 && make install
rm -rf $HOME/ncrystal
cd $CURRENT_DIR
3 changes: 2 additions & 1 deletion tools/ci/gha-install.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
import shutil
import subprocess


def install(omp=False, mpi=False, phdf5=False, dagmc=False, libmesh=False, ncrystal=False):
# Create build directory and change to it
shutil.rmtree('build', ignore_errors=True)
Expand Down Expand Up @@ -42,6 +41,8 @@ def install(omp=False, mpi=False, phdf5=False, dagmc=False, libmesh=False, ncrys

if ncrystal:
cmake_cmd.append('-DOPENMC_USE_NCRYSTAL=ON')
ncrystal_path = os.environ.get('HOME') + '/NCRYSTAL'
cmake_cmd.append(f'-DCMAKE_PREFIX_PATH={ncrystal_path}')

# Build in coverage mode for coverage testing
cmake_cmd.append('-DOPENMC_ENABLE_COVERAGE=on')
Expand Down
5 changes: 2 additions & 3 deletions tools/ci/gha-install.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,8 @@ fi

# Install NCrystal if needed
if [[ $NCRYSTAL = 'y' ]]; then
pip install 'ncrystal>=4.0.0'
#Basic quick verification:
nctool --test
chmod +x ./tools/ci/gha-install-ncrystal.sh
./tools/ci/gha-install-ncrystal.sh
Comment on lines +19 to +20
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't understand why I need to chmod this from the action. I have tried multiple times on my system, and the file is always executable. Maybe someone else can give this a try.

fi

# Install vectfit for WMP generation if needed
Expand Down
4 changes: 2 additions & 2 deletions tools/ci/gha-script.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ if [[ $EVENT == 'y' ]]; then
args="${args} --event "
fi

# Check NCrystal installation
# Install NCrystal for testing
if [[ $NCRYSTAL = 'y' ]]; then
nctool --test
pip install ncrystal
fi

# Run regression and unit tests
Expand Down