Update to spdlog 1.12 and fmt 10.1.1 #473

kkraus14 · 2023-10-16T00:42:39Z

Description

Checklist

I am familiar with the Contributing Guidelines.
New or existing tests cover these changes.
The documentation is up to date with these changes.
The cmake-format.json is up to date with these changes.
I have added new files under rapids-cmake/
- I have added include guards (include_guard(GLOBAL))
- I have added the associated docs/ rst file and update the api.rst

There's an ongoing migration in conda-forge in moving all packages to using spdlog v1.12 and fmt v10. This will eventually cause issues solving environments unless rapids libraries support these versions as well.

Probably need to test downstream libraries especially for the fmt upgrade before we can merge this.

Related RMM issue: rapidsai/rmm#1356
Related conda-forge PR to bring the spdlog patch for nvcc into the 1.12 builds: conda-forge/spdlog-feedstock#59

…dlog 1.12 patch that fixes nvcc

copy-pr-bot · 2023-10-16T00:42:44Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

robertmaynard · 2023-10-16T19:01:50Z

/ok to test

bdice · 2023-10-16T19:08:30Z

@robertmaynard I'll open PRs to test these versions on RAPIDS repos soon. @kkraus14 Thanks for the upstream fix and the backport for conda-forge packages. That helps a lot.

kkraus14 · 2023-10-16T20:28:48Z

FYI -- RMM 23.10 had its tests built successfully in conda-forge: conda-forge/librmm-feedstock#58

bdice · 2023-11-03T01:37:19Z

Testing PRs (edited):

Update to fmt 10.1.1 and spdlog 1.12.0. rmm#1374
- ⚠️ Some changes need to be merged!
Update to fmt 10.1.1 and spdlog 1.12.0. cudf#14355
- ⚠️ Some changes need to be merged!
Update to fmt 10.1.1 and spdlog 1.12.0. ucxx#117
- ⚠️ Some changes need to be merged!
Update to fmt 10.1.1 and spdlog 1.12.0. raft#1957
- ⚠️ Some changes need to be merged!
https://github.com/rapidsai/cumlprims_mg/pull/160
Build with fmt 10.1.1 and spdlog 1.12.0. cuml#5640
nvbench (manually tested)

I think these are all the repos that use spdlog/fmt.

The only downstream user I'm not sure about is nvbench. Do we need to test it too? https://github.com/NVIDIA/nvbench/blob/57c4d42ba505d525a1a3815d2b2560450a4f83dd/cmake/NVBenchDependencies.cmake#L10

robertmaynard · 2023-11-03T01:55:09Z

Yeah we should test nvbench as well. Since it it used by cudf that allows us to test when we gave consumers that use both configurations ( header-only vs library)On Nov 2, 2023, at 9:37 PM, Bradley Dice ***@***.***> wrote: Testing PRs: rapidsai/rmm#1374 rapidsai/cudf#14355 rapidsai/ucxx#117 rapidsai/raft#1957 rapidsai/cuml#5640 I think these are all the RAPIDS repos that use spdlog/fmt. The only downstream user I'm not sure about is nvbench. Do we need to test it too? https://github.com/NVIDIA/nvbench/blob/57c4d42ba505d525a1a3815d2b2560450a4f83dd/cmake/NVBenchDependencies.cmake#L10 —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>

bdice · 2023-11-22T04:31:04Z

Update: conda C++ builds with fmt 10 / spdlog 1.12 are failing for cudf. rapidsai/cudf#14355

I've updated the fmt/spdlog pinnings in rapids-cmake and in cudf, and it's using RMM artifacts with the correct pinnings as well. The conda environment is pulling spdlog 1.12 and fmt 10, so we're getting the build environment that we want. The C++ build shows this failure in the logs:

sccache $BUILD_PREFIX/bin/x86_64-conda-linux-gnu-c++ -DCUFILE_FOUND -DFMT_HEADER_ONLY=1 -DJITIFY_PRINT_LOG=0 -DKVIKIO_CUFILE_BATCH_API_FOUND -DKVIKIO_CUFILE_FOUND -DLIBCUDACXX_ENABLE_EXPERIMENTAL_MEMORY_RESOURCE -DRMM_LOGGING_LEVEL=LIBCUDF_LOGGING_LEVEL -DSPDLOG_ACTIVE_LEVEL=SPDLOG_LEVEL_INFO -DSPDLOG_FMT_EXTERNAL -DTHRUST_DEVICE_SYSTEM=THRUST_DEVICE_SYSTEM_CUDA -DTHRUST_HOST_SYSTEM=THRUST_HOST_SYSTEM_CPP -Dcudf_EXPORTS -I$SRC_DIR/cpp/build/_deps/dlpack-src/include -I$SRC_DIR/cpp/build/_deps/jitify-src -I$SRC_DIR/cpp/include -I$SRC_DIR/cpp/build/include -I$SRC_DIR/cpp/src -I$PREFIX/include/rapids/libcudacxx -I$SRC_DIR/cpp/build/_deps/thrust-src -I$SRC_DIR/cpp/build/_deps/thrust-src/dependencies/cub -I$SRC_DIR/cpp/build/_deps/cuco-src/include -I$BUILD_PREFIX/include -isystem $PREFIX/include/gdeflate -fvisibility-inlines-hidden -fmessage-length=0 -march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O2 -ffunction-sections -pipe -isystem $PREFIX/include -fdebug-prefix-map=$SRC_DIR=/usr/local/src/conda/libcudf-split- -fdebug-prefix-map=$PREFIX=/usr/local/src/conda-prefix  -I$PREFIX/targets/x86_64-linux/include -I$BUILD_PREFIX/targets/x86_64-linux/include  -L$PREFIX/targets/x86_64-linux/lib -L$PREFIX/targets/x86_64-linux/lib/stubs -L$BUILD_PREFIX/targets/x86_64-linux/lib -L$BUILD_PREFIX/targets/x86_64-linux/lib/stubs -O3 -DNDEBUG -std=gnu++17 -fPIC -Wall -Werror -Wno-unknown-pragmas -Wno-error=deprecated-declarations -Wno-deprecated-declarations -pthread -MD -MT CMakeFiles/cudf.dir/src/io/utilities/datasource.cpp.o -MF CMakeFiles/cudf.dir/src/io/utilities/datasource.cpp.o.d -o CMakeFiles/cudf.dir/src/io/utilities/datasource.cpp.o -c $SRC_DIR/cpp/src/io/utilities/datasource.cpp
In file included from $PREFIX/include/spdlog/fmt/fmt.h:31,
                 from $PREFIX/include/spdlog/common.h:50,
                 from $PREFIX/include/spdlog/spdlog.h:12,
                 from $SRC_DIR/cpp/include/cudf/utilities/logger.hpp:19,
                 from $SRC_DIR/cpp/include/cudf/detail/utilities/logger.hpp:19,
                 from $SRC_DIR/cpp/src/io/utilities/config_utils.hpp:18,
                 from $SRC_DIR/cpp/src/io/utilities/datasource.cpp:24:
$PREFIX/include/fmt/core.h: In instantiation of 'constexpr fmt::v10::detail::value<Context> fmt::v10::detail::make_arg(T&) [with bool PACKED = true; Context = fmt::v10::basic_format_context<fmt::v10::appender, char>; T = const cudaError; typename std::enable_if<PACKED, int>::type <anonymous> = 0]':
$PREFIX/include/fmt/core.h:1808:51:   required from 'constexpr fmt::v10::format_arg_store<Context, Args>::format_arg_store(T& ...) [with T = {const cudaError, const char*}; Context = fmt::v10::basic_format_context<fmt::v10::appender, char>; Args = {cudaError, const char*}]'
$PREFIX/include/fmt/core.h:1826:18:   required from 'constexpr fmt::v10::format_arg_store<Context, typename std::remove_cv<typename std::remove_reference<T>::type>::type ...> fmt::v10::make_format_args(T& ...) [with Context = fmt::v10::basic_format_context<fmt::v10::appender, char>; T = {const cudaError, const char*}]'
$PREFIX/include/spdlog/logger.h:374:75:   required from 'void spdlog::logger::log_(spdlog::source_loc, spdlog::level::level_enum, spdlog::string_view_t, Args&& ...) [with Args = {const cudaError&, const char*}; spdlog::string_view_t = fmt::v10::basic_string_view<char>]'
$PREFIX/include/spdlog/logger.h:90:13:   required from 'void spdlog::logger::log(spdlog::source_loc, spdlog::level::level_enum, fmt::v10::format_string<T ...>, Args&& ...) [with Args = {const cudaError&, const char*}; fmt::v10::format_string<T ...> = fmt::v10::basic_format_string<char, const cudaError&, const char*>]'
$SRC_DIR/cpp/src/io/utilities/datasource.cpp:197:7:   required from here
$PREFIX/include/fmt/core.h:1576:63: error: 'fmt::v10::detail::type_is_unformattable_for<const cudaError, char> _' has incomplete type
 1576 |     type_is_unformattable_for<T, typename Context::char_type> _;
      |                                                               ^
$PREFIX/include/fmt/core.h:1580:7: error: static assertion failed: Cannot format an argument. To make type T formattable provide a formatter<T> specialization: https://fmt.dev/latest/api.html#udt
 1580 |       formattable,
      |       ^~~~~~~~~~~
$PREFIX/include/fmt/core.h:1580:7: note: 'formattable' evaluates to false

It seems like we need some C++ updates to make this code compile, but I haven't been able to identify exactly what is needed yet. I think it's failing to format logging messages involving cudaError at compile time? I'll pick up this investigation again soon, but more eyes would be welcome if someone has time.

edit: This was resolved by rapidsai/cudf@bd5077e

bdice · 2023-11-28T18:06:21Z

CPM was having trouble finding fmt because the version was reported incorrectly (10.1.0 instead of 10.1.1). I fixed it with this PR: conda-forge/fmt-feedstock#38.

bdice · 2023-11-28T18:08:53Z

/ok to test

bdice · 2023-11-29T15:03:18Z

CPM is now correctly picking up the fmt package with the version fix. I'm rerunning CI (rmm is complete, now cudf and raft, ...). I'm also attempting to rebuild all of RAPIDS locally with devcontainers.

bdice · 2023-11-30T00:26:38Z

We're making progress! cudf passes CI now. I am rerunning the last few repos. I am hoping to merge this by the end of the week, ideally tomorrow.

bdice

@robertmaynard @kkraus14 I think we can merge this tomorrow. I'm going to do it during work hours because I'm going to request some admin merges on the changes we need to keep from the testing PRs (to conserve CI resources and accelerate the rollout process).

bdice · 2023-11-30T16:05:15Z

All builds succeeded. Reviews have been requested. Merging soon.

bdice · 2023-11-30T16:22:21Z

/merge

This PR updates to fmt 10.1.1 and spdlog 1.12. Depends on rapidsai/rapids-cmake#473. Closes #1356 Authors: - Bradley Dice (https://github.com/bdice) Approvers: - Jake Awe (https://github.com/AyodeAwe) URL: #1374

Upgrade to spdlog 1.12 and fmt 10.1.1, remove fixed fmt patch, add sp…

97e1a5e

…dlog 1.12 patch that fixes nvcc

robertmaynard added feature request New feature or request non-breaking Introduces a non-breaking change labels Oct 16, 2023

bdice changed the base branch from branch-23.12 to branch-24.02 November 16, 2023 05:26

Merge branch 'branch-24.02' into spdlog_1.12_fmt_10

f740fe8

bdice mentioned this pull request Nov 19, 2023

Rebuild for CUDA 12 w/arch + Windows support conda-forge/implicit-feedstock#99

Merged

Merge branch 'branch-24.02' into spdlog_1.12_fmt_10

08223f3

bdice approved these changes Nov 30, 2023

View reviewed changes

bdice mentioned this pull request Nov 30, 2023

Update to fmt 10.1.1 and spdlog 1.12.0. rapidsai/rmm#1374

Merged

bdice self-assigned this Nov 30, 2023

rapids-bot bot merged commit 28a3062 into rapidsai:branch-24.02 Nov 30, 2023
16 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update to spdlog 1.12 and fmt 10.1.1 #473

Update to spdlog 1.12 and fmt 10.1.1 #473

kkraus14 commented Oct 16, 2023 •

edited

Loading

copy-pr-bot bot commented Oct 16, 2023

robertmaynard commented Oct 16, 2023

bdice commented Oct 16, 2023

kkraus14 commented Oct 16, 2023

bdice commented Nov 3, 2023 •

edited

Loading

robertmaynard commented Nov 3, 2023 via email

bdice commented Nov 22, 2023 •

edited

Loading

bdice commented Nov 28, 2023

bdice commented Nov 28, 2023

bdice commented Nov 29, 2023

bdice commented Nov 30, 2023

bdice left a comment

bdice commented Nov 30, 2023

bdice commented Nov 30, 2023

Update to spdlog 1.12 and fmt 10.1.1 #473

Update to spdlog 1.12 and fmt 10.1.1 #473

Conversation

kkraus14 commented Oct 16, 2023 • edited Loading

Description

Checklist

copy-pr-bot bot commented Oct 16, 2023

robertmaynard commented Oct 16, 2023

bdice commented Oct 16, 2023

kkraus14 commented Oct 16, 2023

bdice commented Nov 3, 2023 • edited Loading

robertmaynard commented Nov 3, 2023 via email

bdice commented Nov 22, 2023 • edited Loading

bdice commented Nov 28, 2023

bdice commented Nov 28, 2023

bdice commented Nov 29, 2023

bdice commented Nov 30, 2023

bdice left a comment

Choose a reason for hiding this comment

bdice commented Nov 30, 2023

bdice commented Nov 30, 2023

kkraus14 commented Oct 16, 2023 •

edited

Loading

bdice commented Nov 3, 2023 •

edited

Loading

bdice commented Nov 22, 2023 •

edited

Loading