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

Cannot compile via spack #8

Closed
svenevs opened this issue Oct 17, 2022 · 8 comments
Closed

Cannot compile via spack #8

svenevs opened this issue Oct 17, 2022 · 8 comments
Assignees

Comments

@svenevs
Copy link

svenevs commented Oct 17, 2022

Hello, we are working on the hdf5 cuda integrations for the ECP datavis SDK, I have not been able to find a way to get hdf5-vfd-gds to compile. I've tried a handful of combinations between hdf5 1.13 versions and development branch as well as GCC compilers and cuda versions but haven't found anything successful.

I'm attaching the full build output and will highlight the errors, these errors reproduce whether I'm trying to install the SDK or just install hdf5-vfd-gds (unrelated to the SDK, that is).

spack-build-out.txt

error 1

/tmp/$user/spack-stage/spack-stage-hdf5-vfd-gds-1.0.1-4esgutp6zxhgf5l34q2xcw4yjf6ira75/spack-src/src/H5FDgds.h:22:24: error: initializer element is not computable at load time
   22 | #define H5FD_GDS_NAME  "gds"

error 2:

/tmp/$user/spack-stage/spack-stage-hdf5-vfd-gds-1.0.1-4esgutp6zxhgf5l34q2xcw4yjf6ira75/spack-src/src/H5FDgds.c:310:5: error: incompatible types when initializing type 'enum H5F_close_degree_t' using type 'herr_t (*)(void)' {aka 'int (*)(void)'}
  310 |     H5FD__gds_term,          /* terminate            */

error 3:

/tmp/$user/spack-stage/spack-stage-hdf5-vfd-gds-1.0.1-4esgutp6zxhgf5l34q2xcw4yjf6ira75/spack-src/src/H5FDgds.c:1948:14: error: 'H5FD_CTL__MEM_COPY' undeclared (first use in this function); did you mean 'H5FD_CTL_MEM_COPY'?
 1948 |         case H5FD_CTL__MEM_COPY:

error 4:

/tmp/$user/spack-stage/spack-stage-hdf5-vfd-gds-1.0.1-4esgutp6zxhgf5l34q2xcw4yjf6ira75/spack-src/src/H5FDgds.c:1984:25: error: 'H5FD_CTL__FAIL_IF_UNKNOWN_FLAG' undeclared (first use in this function); did you mean 'H5FD_CTL_FAIL_IF_UNKNOWN_FLAG'?
 1984 |             if (flags & H5FD_CTL__FAIL_IF_UNKNOWN_FLAG)

I took a cursory look but it is not obvious to me what the underlying problem here is. Any thoughts?

@jhendersonHDF
Copy link
Collaborator

Hi @svenevs,

could you try building now with the latest vfd-gds code and using either HDF5 1.13.2 or the develop branch? I just pushed a commit that should update the VFD to work with those branches. If everything goes smoothly I can create a new release.

@svenevs
Copy link
Author

svenevs commented Oct 18, 2022

@jhendersonHDF I was able to test this at two different sites, with hdf5 1.13.2 and hdf5-vfd-gds@master it successfully compiled and installed! Is there anything simple / straightforward to run to test if it actually works? If not I'd just say tag it and bag it -- I'll pin the spack recipe to require your latest release for the SDK 🙂

Thanks!

@jhendersonHDF
Copy link
Collaborator

Is there anything simple / straightforward to run to test if it actually works?

The VFD comes with a small test program that just does a basic read/write with the VFD, but I don't think it's integrated with spack yet, so you might need to build and run it by hand after setting

HDF5_PLUGIN_PATH=/path/to/installed/vfd/library.so

If you have issues with that, I think it's still reasonable to just tag a new version anyway.

@svenevs
Copy link
Author

svenevs commented Oct 19, 2022

@jhendersonHDF AFAICT it worked and produced the gds_simple_dset_write.h5. I think we're ready for a new release tag 🙂

Adding the CMakeLists.txt I was using outside of the project to test, there's some notes about hdf5 (and vfd-gds) cmake quirks. I don't think they should get fixed before a new release, but they were worth noting.

Thanks for your help!

cmake_minimum_required(VERSION 3.18 FATAL_ERROR)
project(hdf5-test LANGUAGES C)

# NOTE: cmake configs are not valid
# 1. produced cmake targets forcibly include cuda toolkit directories
#    (stop using find_package(CUDA) and include directly, use
#    find_package(CUDAToolkit) and just link against CUDA::cudart.
# 2. produced cmake target depends specifically on hdf5-shared, not
#    clear if an issue but the consumer may desire to link against
#    hdf5-static?
find_package(hdf5_vfd_gds CONFIG REQUIRED)
find_package(CUDAToolkit REQUIRED)

# NOTE:
# 3. Seems like a bug that hdf5 isn't bringing MPI... (bug with hdf5 cmake)
find_package(MPI REQUIRED)

add_executable(h5gds_simple_dset_write simple_dset_write.c)
target_link_libraries(h5gds_simple_dset_write
  PUBLIC
    hdf5_vfd_gds
    CUDA::cudart
    MPI::MPI_C)

# NOTE:
# 4. hdf5-vfd-gds cmake target does not include its own headers install directory
#    usually something like
#    target_include_directories(tgt
#       PUBLIC
#         $<BUILD_INTERFACE:...${CMAKE_CURRENT_SOURCE_DIR}/include or something...>
#         $<INSTALL_INTERFACE:include>)  # partially depends on install() logic
#    e.g., CMAKE_INSTALL_INCLUDEDIR could be used rather than hard-coded include
target_include_directories(h5gds_simple_dset_write
  PUBLIC
    /global/u1/s/svenevs/spack_hdf5_cuda/opt/spack/cray-sles15-zen3/gcc-11.2.0/hdf5-vfd-gds-master-bp627tb3l64qewewpkshmswo3n4e3zf7/include)

@jhendersonHDF
Copy link
Collaborator

Thanks for testing @svenevs! I'm currently awaiting a resolution for #5 and then we should be good to go for a new release. Would you mind creating a separate issue for the CMake issues you noted so we don't lose track of it and can close this current issue?

I think I remember the issue with HDF5 not bring in MPI as a dependency, but I'll have to check with others on that one.

@svenevs
Copy link
Author

svenevs commented Oct 19, 2022

@jhendersonHDF sure thing! Redirects:

I'm currently awaiting a resolution for #5

OK, a quick look personally I do not endorse the changes there (the project() call specifically -- you don't have any CUDA source files to compile, nor C++). The other changes seem fine, but not clear why reordering is necessary.

If no resolution is to be had by the end of the week I would encourage tagging a new release before finishing it. That will guarantee we have time to integrate into the ECP SDK before the E4S freeze -- if not this package simply gets yanked (next slot not until February).

If resolution is to be had please @ me and I will re-test compilation from within the SDK!

@svenevs svenevs closed this as completed Oct 19, 2022
@jhendersonHDF
Copy link
Collaborator

Hi @svenevs,

I just released v1.0.2 for your use in spack. Please let me know if you have any issues with this release!

@svenevs
Copy link
Author

svenevs commented Oct 21, 2022

Awesome! It's alive here: spack/spack#33300

Very exciting to get this one in there, GDS is such cool stuff 😎

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

No branches or pull requests

2 participants