Defer cuFile feature checks until finding kvikio package

[bdice]

This PR closes #341.

The kvikio INTERFACE_COMPILE_DEFINITIONS were being set based on the packages available during the libkvikio conda build (e.g. CUDA 12.2 since #328), which might not be the same packages/versions as when libkvikio is actually being used (e.g. to build libcudf with CUDA 12.0). Because we built libkvikio with CUDA 12.2 and then tried to use it with CUDA 12.0 devcontainers, the build failed to find the cuFile Stream APIs that were introduced in CUDA 12.2.

This PR defers these definitions until the call to find_package, which will then use the exact cuFile features present (if cuFile is available at all) when building a package like cudf that depends on kvikio. The libkvikio example/test binary is built with the cuFile features available at build time, for use in the libkvikio-tests conda package. However, this test binary will still be compatible with a runtime where cuFile is unavailable or is version 12.0, as it is dlopen-ing the library and has runtime checks for the batch/stream features it tries to use.

I did local testing of this PR with cudf devcontainers. I tested both 12.0 and 12.2 to reproduce (and fix) the failure, and also tested clean builds of libcudf after removing libcufile (to test when cuFile is not found). All seems to work as intended.

[bdice]

Do we need any changes in the rapids_export(INSTALL kvikio ...) command above? It doesn't seem that we do, from my local testing. I just want to be sure I'm not missing something.

[robertmaynard]

Yes both the BUILD and INSTALL export commands should have the same final code block.

[jakirkham]

Thanks Bradley! 🙏

How would this affect the Python packages built on CUDA 12.2 (especially when they are used on CUDA 12.0)?

[robertmaynard]

Yes both the BUILD and INSTALL export commands should have the same final code block.

[robertmaynard]

You are going to keep adding these defines, includes, link libraries each time find_package is called. We want to do this as rarely as possible,

One option is to wrap the whole check in something along the lines of:

get_property(already_set_kvikio DIRECTORY PROPERTY kvikio_already_set_defines SET)
if(NOT already_set_kvikio)
  set_property(DIRECTORY PROPERTY kvikio_already_set_defines "ON")
   find_package(cuFile)
   ...
endif()

[jakirkham]

How would this affect the Python packages built on CUDA 12.2 (especially when they are used on CUDA 12.0)?

To clarify, what happens if cuDF Python is build against KvikIO on CUDA 12.2 and then runs on CUDA 12.0?

Will we start making use of new symbols from cuFile on CUDA 12.2 that are not available on CUDA 12.0? How do we handle this?

[bdice]

To clarify, what happens if cuDF Python is build against KvikIO on CUDA 12.2 and then runs on CUDA 12.0?

Will we start making use of new symbols from cuFile on CUDA 12.2 that are not available on CUDA 12.0? How do we handle this?

The actual calls to cuFile are guarded by dlopen and conditional checks for the relevant symbols. Building any library that depends on libkvikio with 12.2 and running on 12.0 will still be safe. This PR just makes it possible to build a library depending on libkvikio, such as libcudf, with different CUDA 12.x versions (12.0 or newer), rather than only the CUDA version that was used to build the libkvikio package (which is now 12.2).

[jakirkham]

Are we sure? It looks compile time defined

kvikio/cpp/include/kvikio/shim/cufile.hpp

Lines 93 to 99 in e5bc184

	#ifdef KVIKIO_CUFILE_BATCH_API_FOUND
	get_symbol(BatchIOSetUp, lib, KVIKIO_STRINGIFY(cuFileBatchIOSetUp));
	get_symbol(BatchIOSubmit, lib, KVIKIO_STRINGIFY(cuFileBatchIOSubmit));
	get_symbol(BatchIOGetStatus, lib, KVIKIO_STRINGIFY(cuFileBatchIOGetStatus));
	get_symbol(BatchIOCancel, lib, KVIKIO_STRINGIFY(cuFileBatchIOCancel));
	get_symbol(BatchIODestroy, lib, KVIKIO_STRINGIFY(cuFileBatchIODestroy));
	#endif

[jakirkham]

Should add am ok going ahead with the build improvement

Am more saying there are other issues we might encounter next and maybe we should get out ahead of them

[bdice]

Are we sure? It looks compile time defined

Correct. This PR defers the definition of those macros to the libcudf compilation rather than the libkvikio "compilation" (which is just using CMake to package libkvikio for conda, since libkvikio is header-only).

The problem this solves is that libkvikio conda packages built with CUDA 12.2 assumed the batch/stream features were always available, and defined these macros even when building libcudf with CUDA 12.0 (cuFile from CUDA 12.0 doesn't have batch/stream features). That caused compilation errors. By deferring this check until find_package(KvikIO) is called, we ensure that the library that depends on libkvikio has build-time compatibility rules that are similar to CEC runtime compatibility rules.

If you build libcudf on CUDA 12.0, the batch/stream features aren't built into the library at all (no macros).
If you build libcudf on CUDA 12.2, the batch/stream features will be enabled by the macros and use runtime compatibility checks to determine whether to call those features (12.0 no, 12.2+ yes).

[vyasr]

A couple of small suggestions, but nothing critical.

[jakirkham]

The problem this solves is that libkvikio conda packages built with CUDA 12.2 assumed the batch/stream features were always available, and defined these macros even when building libcudf with CUDA 12.0 (cuFile from CUDA 12.0 doesn't have batch/stream features). That caused compilation errors. By deferring this check until find_package(KvikIO) is called, we ensure that the library that depends on libkvikio has build-time compatibility rules that are similar to CEC runtime compatibility rules.

Yep this is fine

If you build libcudf on CUDA 12.0, the batch/stream features aren't built into the library at all (no macros).
If you build libcudf on CUDA 12.2, the batch/stream features will be enabled by the macros and use runtime compatibility checks to determine whether to call those features (12.0 no, 12.2+ yes).

This is what I was worried about. Though it sounds like it shouldn't be an issue

[bdice]

/merge