2216: Allow `fmt` to be used as external lib #2221

JacobDomagala · 2023-11-28T10:51:35Z

github-actions · 2023-11-28T11:26:53Z

Pipelines results

PR tests (gcc-12, ubuntu, mpich)

Build for 57c2ec8 (2024-04-19 12:06:33 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-9, ubuntu, mpich)

Build for 4a64c6e (2024-04-30 18:38:08 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (gcc-10, ubuntu, openmpi, no LB)

Build for 4a64c6e (2024-04-30 18:38:08 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-11, ubuntu, mpich)

Build for 4a64c6e (2024-04-30 18:38:08 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-12, ubuntu, mpich)

Build for 4a64c6e (2024-04-30 18:38:08 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (nvidia cuda 11.2, gcc-9, ubuntu, mpich)

Build for 4a64c6e (2024-04-30 18:38:08 UTC)

/vt/src/vt/pipe/pipe_manager.impl.h(133): warning: missing return statement at end of non-void function "vt::pipe::PipeManager::makeSend<f,Target>(Target) [with f=&vt::vrt::collection::lb::GreedyLB::collectHandler, Target=vt::objgroup::proxy::ProxyElm<vt::vrt::collection::lb::GreedyLB>]"
          detected during:
            instantiation of "auto vt::pipe::PipeManager::makeSend<f,Target>(Target) [with f=&vt::vrt::collection::lb::GreedyLB::collectHandler, Target=vt::objgroup::proxy::ProxyElm<vt::vrt::collection::lb::GreedyLB>]" 
/vt/src/vt/objgroup/proxy/proxy_objgroup.impl.h(202): here
            instantiation of "vt::objgroup::proxy::Proxy<ObjT>::PendingSendType vt::objgroup::proxy::Proxy<ObjT>::reduce<f,Op,Target,Args...>(Target, Args &&...) const [with ObjT=vt::vrt::collection::lb::GreedyLB, f=&vt::vrt::collection::lb::GreedyLB::collectHandler, Op=vt::collective::PlusOp, Target=vt::objgroup::proxy::ProxyElm<vt::vrt::collection::lb::GreedyLB>, Args=<vt::vrt::collection::lb::GreedyPayload>]" 
/vt/src/vt/vrt/collection/balance/greedylb/greedylb.cc(222): here

/vt/src/vt/pipe/pipe_manager.impl.h(133): warning: missing return statement at end of non-void function "vt::pipe::PipeManager::makeSend<f,Target>(Target) [with f=&MyObj::handler, Target=vt::objgroup::proxy::ProxyElm<MyObj>]"
          detected during instantiation of "auto vt::pipe::PipeManager::makeSend<f,Target>(Target) [with f=&MyObj::handler, Target=vt::objgroup::proxy::ProxyElm<MyObj>]" 
/vt/examples/callback/callback.cc(147): here

/vt/src/vt/pipe/pipe_manager.impl.h(133): warning: missing return statement at end of non-void function "vt::pipe::PipeManager::makeSend<f,Target>(Target) [with f=&colHan, Target=vt::vrt::collection::VrtElmProxy<MyCol, vt::Index1D>]"
          detected during instantiation of "auto vt::pipe::PipeManager::makeSend<f,Target>(Target) [with f=&colHan, Target=vt::vrt::collection::VrtElmProxy<MyCol, vt::Index1D>]" 
/vt/examples/callback/callback.cc(153): here

/vt/src/vt/pipe/pipe_manager.impl.h(133): warning: missing return statement at end of non-void function "vt::pipe::PipeManager::makeSend<f,Target>(Target) [with f=&MyObj::handler, Target=vt::objgroup::proxy::ProxyElm<MyObj>]"
          detected during instantiation of "auto vt::pipe::PipeManager::makeSend<f,Target>(Target) [with f=&MyObj::handler, Target=vt::objgroup::proxy::ProxyElm<MyObj>]" 
/vt/examples/callback/callback.cc(147): here

/vt/src/vt/pipe/pipe_manager.impl.h(133): warning: missing return statement at end of non-void function "vt::pipe::PipeManager::makeSend<f,Target>(Target) [with f=&colHan, Target=vt::vrt::collection::VrtElmProxy<MyCol, vt::Index1D>]"
          detected during instantiation of "auto vt::pipe::PipeManager::makeSend<f,Target>(Target) [with f=&colHan, Target=vt::vrt::collection::VrtElmProxy<MyCol, vt::Index1D>]" 
/vt/examples/callback/callback.cc(153%0D%0A%0D%0A%0D%0A ==> And there is more. Read log. <==

Build log

PR tests (gcc-9, ubuntu, mpich, zoltan, json schema test)

Build for 4a64c6e (2024-04-30 18:38:08 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-13, ubuntu, mpich)

Build for 4a64c6e (2024-04-30 18:38:08 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-14, ubuntu, mpich)

Build for 57c2ec8 (2024-04-19 12:06:33 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-10, ubuntu, mpich)

Build for 4a64c6e (2024-04-30 18:38:08 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (gcc-11, ubuntu, mpich, trace runtime, coverage)

Build for 4a64c6e (2024-04-30 18:38:08 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (gcc-8, ubuntu, mpich, address sanitizer)

Build for 4a64c6e (2024-04-30 18:38:08 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (intel icpc, ubuntu, mpich)

Build for 4a64c6e (2024-04-30 18:38:08 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-13, alpine, mpich)

Build for 4a64c6e (2024-04-30 18:38:08 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (intel icpx, ubuntu, mpich)

Build for 57c2ec8 (2024-04-19 12:06:33 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (nvidia cuda 12.2.0, gcc-9, ubuntu, mpich)

Build for 57c2ec8 (2024-04-19 12:06:33 UTC)

/vt/lib/CLI/CLI/CLI11.hpp(1029): warning #2361-D: invalid narrowing conversion from "double" to "unsigned long"
          TT { std::declval<CC>() }
               ^
          detected during:
            instantiation of "vt::CLI::detail::is_direct_constructible<T, C>::test [with T=std::vector<std::string, std::allocator<std::string>>, C=double]" based on template arguments <std::vector<std::string, std::allocator<std::string>>, double> at line 1041
            instantiation of class "vt::CLI::detail::is_direct_constructible<T, C> [with T=std::vector<std::string, std::allocator<std::string>>, C=double]" at line 5005
            instantiation of "void vt::CLI::Option::results(T &) const [with T=std::vector<std::string, std::allocator<std::string>>]" at line 5034
            instantiation of "T vt::CLI::Option::as<T>() const [with T=std::vector<std::string, std::allocator<std::string>>]" at line 7315

Remark: The warnings can be suppressed with "-diag-suppress <warning-number>"

/vt/lib/CLI/CLI/CLI11.hpp(1029): warning #2361-D: invalid narrowing conversion from "int" to "unsigned long"
          TT { std::declval<CC>() }
               ^
          detected during:
            instantiation of "vt::CLI::detail::is_direct_constructible<T, C>::test [with T=std::vector<std::string, std::allocator<std::string>>, C=int]" based on template arguments <std::vector<std::string, std::allocator<std::string>>, int> at line 1041
            instantiation of class "vt::CLI::detail::is_direct_constructible<T, C> [with T=std::vector<std::string, std::allocator<std::string>>, C=int]" at line 5005
            instantiation of "void vt::CLI::Option::results(T &) const [with T=std::vector<std::string, std::allocator<std::string>>]" at line 5034
            instantiation of "T vt::CLI::Option::as<T>() const [with T=std::vector<std::string, std::allocator<std::string>>]" at line 7315

Testing - passed

Build log

PR tests (intel icpx, ubuntu, mpich, verbose)

Build for 4a64c6e (2024-04-30 18:38:08 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (gcc-12, ubuntu, mpich, verbose)

Build for 4a64c6e (2024-04-30 18:38:08 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-14, ubuntu, mpich, verbose)

Build for 4a64c6e (2024-04-30 18:38:08 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (nvidia cuda 12.2.0, gcc-9, ubuntu, mpich, verbose)

Build for 4a64c6e (2024-04-30 18:38:08 UTC)

/vt/lib/CLI/CLI/CLI11.hpp(1029): warning #2361-D: invalid narrowing conversion from "double" to "unsigned long"
          TT { std::declval<CC>() }
               ^
          detected during:
            instantiation of "vt::CLI::detail::is_direct_constructible<T, C>::test [with T=std::vector<std::string, std::allocator<std::string>>, C=double]" based on template arguments <std::vector<std::string, std::allocator<std::string>>, double> at line 1041
            instantiation of class "vt::CLI::detail::is_direct_constructible<T, C> [with T=std::vector<std::string, std::allocator<std::string>>, C=double]" at line 5005
            instantiation of "void vt::CLI::Option::results(T &) const [with T=std::vector<std::string, std::allocator<std::string>>]" at line 5034
            instantiation of "T vt::CLI::Option::as<T>() const [with T=std::vector<std::string, std::allocator<std::string>>]" at line 7315

Remark: The warnings can be suppressed with "-diag-suppress <warning-number>"

/vt/lib/CLI/CLI/CLI11.hpp(1029): warning #2361-D: invalid narrowing conversion from "int" to "unsigned long"
          TT { std::declval<CC>() }
               ^
          detected during:
            instantiation of "vt::CLI::detail::is_direct_constructible<T, C>::test [with T=std::vector<std::string, std::allocator<std::string>>, C=int]" based on template arguments <std::vector<std::string, std::allocator<std::string>>, int> at line 1041
            instantiation of class "vt::CLI::detail::is_direct_constructible<T, C> [with T=std::vector<std::string, std::allocator<std::string>>, C=int]" at line 5005
            instantiation of "void vt::CLI::Option::results(T &) const [with T=std::vector<std::string, std::allocator<std::string>>]" at line 5034
            instantiation of "T vt::CLI::Option::as<T>() const [with T=std::vector<std::string, std::allocator<std::string>>]" at line 7315

Testing - passed

Build log

JacobDomagala · 2023-12-13T20:35:47Z

Looks like newer versions of fmt start to cause issues on nvcc build. I've tested it locally and they're gone on version 12.3.

Given that we probably want to support 11.2 for a while, we could fix them with using external fmt (7.1.3 version), at least to satisfy CIs, or should we rollback the bundled version to 7.1.3? 🤔

JacobDomagala · 2023-12-19T17:24:32Z

I updated the CUDA pipeline to use external fmt (version 7.1.3)

JacobDomagala · 2023-12-21T11:43:25Z

@lifflander Rolled back fmt to 7.1.3

lifflander · 2024-04-17T22:04:28Z

@nmm0 I believe we are waiting on your review for this.

nmm0

We need to add the following in vtConfig.cmake.in:

if (@vt_external_fmt@)
  set (fmt_DIR @fmt_DIR@)
  find_dependency(fmt REQUIRED HINTS @fmt_DIR@)
endif()

So that the dependency will find fmt at the same place that vt had it

nmm0 · 2024-02-20T17:34:11Z

cmake/load_bundled_libraries.cmake

+# use included fmt or external one
+if(${vt_external_fmt})
+  # user should provide 'fmt_DIR' to CMake (unless fmt is installed in system libs)
+  if(fmt_DIR)


fmt_ROOT should also be valid here

JacobDomagala · 2024-04-19T23:14:57Z

Thanks @nmm0 for the review!
I updated the PR. Had to make few changes to source code, as the previous version wasn't compiling with external fmt (with most recent version of fmt).

nmm0

Looks good to me, thank you for doing this :)

src/vt/collective/reduce/reduce_manager.h

lifflander · 2024-04-30T17:01:18Z

@JacobDomagala Can you rebase this PR?

…ersions. Make 9.1.0 fmt version as required for external fmt

…e on dockerhub

JacobDomagala self-assigned this Nov 28, 2023

JacobDomagala linked an issue Nov 28, 2023 that may be closed by this pull request

fmt library incompatibilities #2216

Closed

JacobDomagala force-pushed the 2216-fmt-library-incompatibilities branch from a5963f1 to d537fd6 Compare November 28, 2023 12:10

JacobDomagala force-pushed the 2216-fmt-library-incompatibilities branch 4 times, most recently from 4dbe64a to bf7cce1 Compare December 12, 2023 17:48

JacobDomagala marked this pull request as ready for review December 12, 2023 17:49

JacobDomagala requested review from lifflander, nmm0, thearusable and nlslatt December 12, 2023 17:49

JacobDomagala force-pushed the 2216-fmt-library-incompatibilities branch 2 times, most recently from 807f2e7 to a97d9fa Compare December 12, 2023 20:34

JacobDomagala force-pushed the 2216-fmt-library-incompatibilities branch 3 times, most recently from 1783629 to 9f8dd40 Compare December 19, 2023 10:12

JacobDomagala force-pushed the 2216-fmt-library-incompatibilities branch 3 times, most recently from 0227d7e to 4bf6694 Compare December 20, 2023 23:46

JacobDomagala mentioned this pull request Dec 21, 2023

#2175: Add YAML as input as alternative to command-line arguments #2230

Merged

JacobDomagala force-pushed the 2216-fmt-library-incompatibilities branch from 4bf6694 to 0da91f5 Compare January 16, 2024 12:34

JacobDomagala changed the title ~~2216: Update fmt lib~~ 2216: Allow 'fmt' to be used as external lib Jan 16, 2024

JacobDomagala changed the title ~~2216: Allow 'fmt' to be used as external lib~~ 2216: Allow fmt to be used as external lib Jan 16, 2024

JacobDomagala force-pushed the 2216-fmt-library-incompatibilities branch from 0da91f5 to e637943 Compare January 23, 2024 15:59

JacobDomagala force-pushed the 2216-fmt-library-incompatibilities branch from e637943 to f1ec391 Compare January 30, 2024 17:24

JacobDomagala force-pushed the 2216-fmt-library-incompatibilities branch 2 times, most recently from afb65d6 to 34c5cbc Compare February 20, 2024 17:00

nmm0 requested changes Apr 18, 2024

View reviewed changes

JacobDomagala force-pushed the 2216-fmt-library-incompatibilities branch from 34c5cbc to 57c2ec8 Compare April 19, 2024 12:06

JacobDomagala force-pushed the 2216-fmt-library-incompatibilities branch from 57c2ec8 to 1489dd1 Compare April 23, 2024 16:16

nmm0 approved these changes Apr 23, 2024

View reviewed changes

src/vt/collective/reduce/reduce_manager.h Show resolved Hide resolved

JacobDomagala added 15 commits April 30, 2024 20:38

#2216: Update fmt to 10.1

66dd13a

#2216: Provide custom fmt::formatters for various vt types

21c6d6b

#2216: Add CMake option to enable using external fmt

1a166e0

#2216: Remove fmt::runtime

f96538e

#2216: Use macro for using correct fmt headers

76ab9da

#2216: Fix issue with namespace when using external and bundled fmt v…

6e52faf

…ersions. Make 9.1.0 fmt version as required for external fmt

#2216: Fix namespace issues and remove uneeded custom formatters

963ad18

#2216: Add script for building external fmt

80ac01c

#2216: Build external fmt for CUDA pipeline

ba147c7

#2216: Bump version of cuda image, since 11.2.0 is no longer availabl…

414b34e

…e on dockerhub

#2216: Update build_vt_sample.sh to use external fmt when it's requested

6580f4b

#2216: Rollback fmt version to 7.1.3 due to CUDA

c2f090e

#2216: Fix compile errors with newer version of fmt

fae07f8

#2216: Address review comments

3585c71

#2216: Fix compile errors when using newer fmt version

4a64c6e

JacobDomagala force-pushed the 2216-fmt-library-incompatibilities branch from 1489dd1 to 4a64c6e Compare April 30, 2024 18:38

lifflander merged commit a4a29a2 into develop Apr 30, 2024
26 checks passed

cwschilly mentioned this pull request Jul 10, 2024

Add support for external yaml-cpp lib #2322

Open

cwschilly mentioned this pull request Aug 6, 2024

Add support for external fmt library DARMA-tasking/vt-tv#102

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

2216: Allow `fmt` to be used as external lib #2221

2216: Allow `fmt` to be used as external lib #2221

JacobDomagala commented Nov 28, 2023

github-actions bot commented Nov 28, 2023 •

edited

Loading

JacobDomagala commented Dec 13, 2023 •

edited

Loading

JacobDomagala commented Dec 19, 2023

JacobDomagala commented Dec 21, 2023

lifflander commented Apr 17, 2024

nmm0 left a comment

nmm0 Feb 20, 2024

JacobDomagala commented Apr 19, 2024

nmm0 left a comment

lifflander commented Apr 30, 2024

2216: Allow fmt to be used as external lib #2221

2216: Allow fmt to be used as external lib #2221

Conversation

JacobDomagala commented Nov 28, 2023

github-actions bot commented Nov 28, 2023 • edited Loading

Pipelines results

JacobDomagala commented Dec 13, 2023 • edited Loading

JacobDomagala commented Dec 19, 2023

JacobDomagala commented Dec 21, 2023

lifflander commented Apr 17, 2024

nmm0 left a comment

Choose a reason for hiding this comment

nmm0 Feb 20, 2024

Choose a reason for hiding this comment

JacobDomagala commented Apr 19, 2024

nmm0 left a comment

Choose a reason for hiding this comment

lifflander commented Apr 30, 2024

2216: Allow `fmt` to be used as external lib #2221

2216: Allow `fmt` to be used as external lib #2221

github-actions bot commented Nov 28, 2023 •

edited

Loading

JacobDomagala commented Dec 13, 2023 •

edited

Loading