Skip to content

Conversation

@bvanessen
Copy link
Collaborator

Adding additional HDF5 plugins and ZFP to the superbuild. Updating
the cuda-distconv.sh superbuild example.

@bvanessen bvanessen requested a review from benson31 March 11, 2024 14:58
Comment on lines +27 to +35
# if (TARGET HDF5 AND NOT LBANN_SB_FWD_ZFP_HDF5_DIR)
# set(LBANN_SB_FWD_ZFP_HDF5_DIR ${HDF5_DIR})
# endif ()

# Conduit is "cute" about finding HDF5. It's not a CMake option() --
# you opt in by setting HDF5_DIR explicitly. So let's do that.
# if (TARGET HDF5 AND NOT LBANN_SB_FWD_ZFP_HDF5_DIR)
# set(LBANN_SB_FWD_ZFP_HDF5_DIR ${HDF5_DIR})
# endif ()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
# if (TARGET HDF5 AND NOT LBANN_SB_FWD_ZFP_HDF5_DIR)
# set(LBANN_SB_FWD_ZFP_HDF5_DIR ${HDF5_DIR})
# endif ()
# Conduit is "cute" about finding HDF5. It's not a CMake option() --
# you opt in by setting HDF5_DIR explicitly. So let's do that.
# if (TARGET HDF5 AND NOT LBANN_SB_FWD_ZFP_HDF5_DIR)
# set(LBANN_SB_FWD_ZFP_HDF5_DIR ${HDF5_DIR})
# endif ()

Comment on lines +40 to +51
# option(LBANN_SB_FWD_HDF5_HDF5_USE_16_API_DEFAULT
# "Use 1.6 API by default"
# OFF)
# option(LBANN_SB_FWD_HDF5_HDF5_USE_18_API_DEFAULT
# "Use 1.8 API by default"
# OFF)
# option(LBANN_SB_FWD_HDF5_HDF5_USE_110_API_DEFAULT
# "Use 1.10 API by default"
# OFF)
# option(LBANN_SB_FWD_HDF5_HDF5_USE_112_API_DEFAULT
# "Use 1.12 API by default"
# ON)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
# option(LBANN_SB_FWD_HDF5_HDF5_USE_16_API_DEFAULT
# "Use 1.6 API by default"
# OFF)
# option(LBANN_SB_FWD_HDF5_HDF5_USE_18_API_DEFAULT
# "Use 1.8 API by default"
# OFF)
# option(LBANN_SB_FWD_HDF5_HDF5_USE_110_API_DEFAULT
# "Use 1.10 API by default"
# OFF)
# option(LBANN_SB_FWD_HDF5_HDF5_USE_112_API_DEFAULT
# "Use 1.12 API by default"
# ON)

Comment on lines +61 to +63
# option(LBANN_SB_FWD_HDF5_DEFAULT_API_VERSION
# "Only Build Shared Libraries."
# v112)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
# option(LBANN_SB_FWD_HDF5_DEFAULT_API_VERSION
# "Only Build Shared Libraries."
# v112)

Comment on lines +27 to +31
# Conduit is "cute" about finding HDF5. It's not a CMake option() --
# you opt in by setting HDF5_DIR explicitly. So let's do that.
# if (TARGET HDF5 AND NOT LBANN_SB_FWD_ZFP_HDF5_DIR)
# set(LBANN_SB_FWD_ZFP_HDF5_DIR ${HDF5_DIR})
# endif ()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
# Conduit is "cute" about finding HDF5. It's not a CMake option() --
# you opt in by setting HDF5_DIR explicitly. So let's do that.
# if (TARGET HDF5 AND NOT LBANN_SB_FWD_ZFP_HDF5_DIR)
# set(LBANN_SB_FWD_ZFP_HDF5_DIR ${HDF5_DIR})
# endif ()

Comment on lines +118 to +122
set(LBANN_SB_SUGG_CMAKE_PREFIX_PATH_TMP "${CMAKE_PREFIX_PATH}:\$\{CMAKE_PREFIX_PATH\}")
message("BVE I think that there is a prefix path ${CMAKE_PREFIX_PATH}")
message("BVE I think that there is a foo ${FOO}")
message("BVE I think that there is a suggesed prefix path ${LBANN_SB_SUGG_CMAKE_PREFIX_PATH_TMP}")
#set(LBANN_SB_SUGG_CMAKE_PREFIX_PATH_TMP "\$\{CMAKE_PREFIX_PATH\}")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
set(LBANN_SB_SUGG_CMAKE_PREFIX_PATH_TMP "${CMAKE_PREFIX_PATH}:\$\{CMAKE_PREFIX_PATH\}")
message("BVE I think that there is a prefix path ${CMAKE_PREFIX_PATH}")
message("BVE I think that there is a foo ${FOO}")
message("BVE I think that there is a suggesed prefix path ${LBANN_SB_SUGG_CMAKE_PREFIX_PATH_TMP}")
#set(LBANN_SB_SUGG_CMAKE_PREFIX_PATH_TMP "\$\{CMAKE_PREFIX_PATH\}")

# Set to the preferred build directory
BUILD_DIR=${TMPDIR}/lbann-superbuild

# -G Ninja \
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
# -G Ninja \

# -G Ninja \
cmake \
-G Ninja \
-G "Unix Makefiles" \
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
-G "Unix Makefiles" \

-D LBANN_SB_Aluminum_CXX_FLAGS="${EXTRA_CXX_FLAGS}" \
-D LBANN_SB_Aluminum_CUDA_FLAGS="${EXTRA_CUDA_FLAGS}" \
-D LBANN_SB_FWD_Aluminum_ALUMINUM_ENABLE_CALIPER=ON \
-D LBANN_SB_FWD_Aluminum_ALUMINUM_ENABLE_CALIPER=OFF \
Copy link
Collaborator

Choose a reason for hiding this comment

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

May as well set LBANN_SB_BUILD_adiak=OFF and LBANN_SB_BUILD_Caliper=OFF, too.

Comment on lines +80 to +82
ZFP # LBANN
H5Z-ZFP # LBANN
HDF5_PLUGINS # LBANN
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
ZFP # LBANN
H5Z-ZFP # LBANN
HDF5_PLUGINS # LBANN
ZFP # LBANN (runtime)
H5Z-ZFP # LBANN (runtime)
HDF5_PLUGINS # LBANN (runtime)

Copy link
Collaborator

Choose a reason for hiding this comment

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

This file is a bit of a mess -- it looks like this is just a copy of the cacheinit.cmake file you load with -C later on? I'd prefer to just use that cacheinit file rather than have a full copy here. If there are still a bunch of variables you'd prefer to specify manually, I'd prefer to still use a separate file (see the opencv recipe for an example of this).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also, this was not executable in the repository on purpose (executing it wouldn't preserve the meaningful export statements) -- it should only ever be sourced.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants