Skip to content
Snippets Groups Projects

back to CMAKE_CXX_STANDARD 14

Closed back to CMAKE_CXX_STANDARD 14
plyushchenko-muon-table-reading into master
Closed Daniil Pliushchenko requested to merge plyushchenko-muon-table-reading into master

Looks like the project doesn't compile with ROOT with CMAKE_CXX_STANDARD=17 . https://gitlab.cern.ch/lhcb-parallelization/Allen/merge_requests/122#note_2577137

In this MR I moved it back to 14, also got rid of c++17 code.

@freiss Please let me know whether it works for you.

Edited by Daniil Pliushchenko

Merge request reports

Approval is optional

Closed by Daniel Hugo Campora PerezDaniel Hugo Campora Perez 5 years ago (May 14, 2019 8:17am UTC)

Merge details

  • The changes were not merged into master.

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Daniil Pliushchenko mentioned in merge request !122 (merged)

    mentioned in merge request !122 (merged)

  • Daniil Pliushchenko marked as a Work In Progress

    marked as a Work In Progress

  • Daniil Pliushchenko changed the description

    changed the description

  • Daniil Pliushchenko unmarked as a Work In Progress

    unmarked as a Work In Progress

  • Daniil Pliushchenko changed the description

    changed the description

  • Flo Reiss
    Flo Reiss @freiss ·
    Developer

    this works for me again. Might be worth to figure out what the problem was, though

  • D
    Daniil Pliushchenko @dpliushc ·
    Author Contributor

    @dcampora Could you please merge this MR? Seems to be the quickest way to get rid of c++17-related problems for now

  • D
    Dorothea Vom Bruch @dovombru ·
    Maintainer

    I saw these issues also in the MR by @raaij on non-event data (MR 105). It uses C++17, but does not compile with ROOT for me.

  • Daniel Hugo Campora Perez
    Daniel Hugo Campora Perez @dcampora ·
    Maintainer

    @freiss could you please detail with a dump in this thread what version of ROOT and what exactly is the error?

    CC @clemenci

  • C
    Marco Clemencic @clemenci ·
    Owner

    To use C++17 you need a version of ROOT built with C++17 (as a rule of thumb: do not choose a version of the standard, just pick up what ROOT uses).

  • Flo Reiss
    Flo Reiss @freiss ·
    Developer

    using gcc-7, ROOT 6.16 built with c++17 enabled on my laptop:

    [10/81] Building CUDA object stream/CMakeFiles/Stream.dir/sequence/src/Stream.cu.o
FAILED: stream/CMakeFiles/Stream.dir/sequence/src/Stream.cu.o 
/usr/local/cuda/bin/nvcc  -DWITH_ROOT -I/usr/local/include -I../stream/gear/include -I../stream/memory_manager/include -I../stream/scheduler/include -I../stream/sequence/include -I../stream/setup/include -I../stream/checkers/include -I../main/include -I../cuda/utils/prefix_sum/include -I../cuda/utils/float_operations/include -I../cuda/utils/sorting/include -I../cuda/utils/binary_search/include -I../cuda/event_model/velo/include -I../cuda/event_model/common/include -I../cuda/event_model/UT/include -I../cuda/event_model/SciFi/include -I../cuda/event_model/associate/include -I../cuda/global_event_cut/include -I../cuda/ip_cut/include -I../cuda/UT/common/include -I../cuda/UT/PrVeloUT/include -I../cuda/UT/UTDecoding/include -I../cuda/UT/sorting/include -I../cuda/UT/consolidate/include -I../cuda/UT/compassUT/include -I../cuda/velo/common/include -I../cuda/velo/calculate_phi_and_sort/include -I../cuda/velo/consolidate_tracks/include -I../cuda/velo/mask_clustering/include -I../cuda/velo/prefix_sum/include -I../cuda/velo/search_by_triplet/include -I../cuda/velo/simplified_kalman_filter/include -I../cuda/SciFi/preprocessing/include -I../cuda/SciFi/common/include -I../cuda/SciFi/PrForward/include -I../cuda/SciFi/looking_forward/common/include -I../cuda/SciFi/looking_forward/calculate_first_layer_window/include -I../cuda/SciFi/looking_forward/calculate_second_layer_window/include -I../cuda/SciFi/looking_forward/form_seeds_from_candidates/include -I../cuda/SciFi/looking_forward/calculate_candidate_extrapolation_window/include -I../cuda/SciFi/looking_forward/promote_candidates/include -I../cuda/SciFi/looking_forward/calculate_track_extrapolation_window/include -I../cuda/SciFi/looking_forward/extend_tracks/include -I../cuda/SciFi/looking_forward_sbt/search_initial_windows/include -I../cuda/SciFi/looking_forward_sbt/collect_candidates/include -I../cuda/SciFi/looking_forward_sbt/find_compatible_windows/include -I../cuda/SciFi/looking_forward_sbt/triplet_seeding/include -I../cuda/SciFi/looking_forward_sbt/triplet_keep_best/include -I../cuda/SciFi/looking_forward_sbt/extend_tracks_x/include -I../cuda/SciFi/looking_forward_sbt/composite_algorithms/include -I../cuda/SciFi/looking_forward_sbt/extend_tracks_first_layers_x/include -I../cuda/SciFi/looking_forward_sbt/extend_tracks_uv/include -I../cuda/SciFi/looking_forward_sbt/quality_filter/include -I../cuda/SciFi/looking_forward_sbt/quality_filter_x/include -I../cuda/SciFi/looking_forward_sbt/search_uv_windows/include -I../cuda/SciFi/looking_forward_sbt/fit/include -I../cuda/SciFi/consolidate/include -I../cuda/associate/include -I../cuda/muon/common/include -I../cuda/muon/preprocessing/include -I../cuda/muon/is_muon/include -I../cuda/muon/classification/include -I../cuda/muon/decoding/include -I../cuda/utils/include -I../cuda/PV/patPV/include -I../cuda/PV/common/include -I../x86/muon/decoding/include -I../x86/velo/clustering/include -I../x86/UT/PrVeloUT/include -I../x86/SciFi/PrForward/include -I../x86/SciFi/LookingForward/include -I../x86/SciFi/MomentumForward/include -I../x86/PV/beamlinePV/include -I../x86/utils/prefix_sum/include -I../x86/global_event_cut/include -I../checker/tracking/include -I../checker/pv/include -I../cuda/PV/beamlinePV/include -I../cuda/kalman/ParKalman/include -Iconfiguration/sequences -I../mdf/include -arch=sm_50 --use_fast_math --expt-relaxed-constexpr --maxrregcount=63  -O3 -g -DNDEBUG --generate-line-info   -std=c++14 -x cu -dc ../stream/sequence/src/Stream.cu -o stream/CMakeFiles/Stream.dir/sequence/src/Stream.cu.o && /usr/local/cuda/bin/nvcc  -DWITH_ROOT -I/usr/local/include -I../stream/gear/include -I../stream/memory_manager/include -I../stream/scheduler/include -I../stream/sequence/include -I../stream/setup/include -I../stream/checkers/include -I../main/include -I../cuda/utils/prefix_sum/include -I../cuda/utils/float_operations/include -I../cuda/utils/sorting/include -I../cuda/utils/binary_search/include -I../cuda/event_model/velo/include -I../cuda/event_model/common/include -I../cuda/event_model/UT/include -I../cuda/event_model/SciFi/include -I../cuda/event_model/associate/include -I../cuda/global_event_cut/include -I../cuda/ip_cut/include -I../cuda/UT/common/include -I../cuda/UT/PrVeloUT/include -I../cuda/UT/UTDecoding/include -I../cuda/UT/sorting/include -I../cuda/UT/consolidate/include -I../cuda/UT/compassUT/include -I../cuda/velo/common/include -I../cuda/velo/calculate_phi_and_sort/include -I../cuda/velo/consolidate_tracks/include -I../cuda/velo/mask_clustering/include -I../cuda/velo/prefix_sum/include -I../cuda/velo/search_by_triplet/include -I../cuda/velo/simplified_kalman_filter/include -I../cuda/SciFi/preprocessing/include -I../cuda/SciFi/common/include -I../cuda/SciFi/PrForward/include -I../cuda/SciFi/looking_forward/common/include -I../cuda/SciFi/looking_forward/calculate_first_layer_window/include -I../cuda/SciFi/looking_forward/calculate_second_layer_window/include -I../cuda/SciFi/looking_forward/form_seeds_from_candidates/include -I../cuda/SciFi/looking_forward/calculate_candidate_extrapolation_window/include -I../cuda/SciFi/looking_forward/promote_candidates/include -I../cuda/SciFi/looking_forward/calculate_track_extrapolation_window/include -I../cuda/SciFi/looking_forward/extend_tracks/include -I../cuda/SciFi/looking_forward_sbt/search_initial_windows/include -I../cuda/SciFi/looking_forward_sbt/collect_candidates/include -I../cuda/SciFi/looking_forward_sbt/find_compatible_windows/include -I../cuda/SciFi/looking_forward_sbt/triplet_seeding/include -I../cuda/SciFi/looking_forward_sbt/triplet_keep_best/include -I../cuda/SciFi/looking_forward_sbt/extend_tracks_x/include -I../cuda/SciFi/looking_forward_sbt/composite_algorithms/include -I../cuda/SciFi/looking_forward_sbt/extend_tracks_first_layers_x/include -I../cuda/SciFi/looking_forward_sbt/extend_tracks_uv/include -I../cuda/SciFi/looking_forward_sbt/quality_filter/include -I../cuda/SciFi/looking_forward_sbt/quality_filter_x/include -I../cuda/SciFi/looking_forward_sbt/search_uv_windows/include -I../cuda/SciFi/looking_forward_sbt/fit/include -I../cuda/SciFi/consolidate/include -I../cuda/associate/include -I../cuda/muon/common/include -I../cuda/muon/preprocessing/include -I../cuda/muon/is_muon/include -I../cuda/muon/classification/include -I../cuda/muon/decoding/include -I../cuda/utils/include -I../cuda/PV/patPV/include -I../cuda/PV/common/include -I../x86/muon/decoding/include -I../x86/velo/clustering/include -I../x86/UT/PrVeloUT/include -I../x86/SciFi/PrForward/include -I../x86/SciFi/LookingForward/include -I../x86/SciFi/MomentumForward/include -I../x86/PV/beamlinePV/include -I../x86/utils/prefix_sum/include -I../x86/global_event_cut/include -I../checker/tracking/include -I../checker/pv/include -I../cuda/PV/beamlinePV/include -I../cuda/kalman/ParKalman/include -Iconfiguration/sequences -I../mdf/include -arch=sm_50 --use_fast_math --expt-relaxed-constexpr --maxrregcount=63  -O3 -g -DNDEBUG --generate-line-info   -std=c++14 -x cu -M ../stream/sequence/src/Stream.cu -MT stream/CMakeFiles/Stream.dir/sequence/src/Stream.cu.o -o stream/CMakeFiles/Stream.dir/sequence/src/Stream.cu.o.d
In file included from /usr/local/include/TString.h:28:0,
                 from /usr/local/include/TNamed.h:26,
                 from /usr/local/include/TDirectory.h:25,
                 from ../checker/tracking/include/TrackChecker.h:27,
                 from ../stream/sequence/include/HostBuffers.cuh:8,
                 from ../stream/sequence/include/Stream.cuh:19,
                 from ../stream/sequence/src/Stream.cu:1:
/usr/local/include/ROOT/RStringView.hxx:19:23: fatal error: string_view: No such file or directory
 #include <string_view>

    What is strange to me, is that I can do #include <string_view> in an interactive ROOT session without any problems

  • C
    Marco Clemencic @clemenci ·
    Owner

    It looks like you didn't tell nvcc to use c++17.

  • Flo Reiss
    Flo Reiss @freiss ·
    Developer

    changing the flag to -std=c++17 does not seem to work: nvcc fatal : Value 'c++17' is not defined for option 'std'

  • Daniel Hugo Campora Perez
    Daniel Hugo Campora Perez @dcampora ·
    Maintainer

    nvcc does not support C++17 yet.

  • Flo Reiss
    Flo Reiss @freiss ·
    Developer

    ok, I figured as much. What is the way forward here?

  • Daniel Hugo Campora Perez
    Daniel Hugo Campora Perez @dcampora ·
    Maintainer

    After a bit of though, we require ROOT in principle only in:

    • CPU algorithms of the sequence
    • Checkers

    The checkers can easily be compiled with the host compiler, if they are not already.

    I am not sure what the implications for the CPU algos would be without trying. They need to be embedded in a Handler as they are part of the sequence, but they are in another compilation unit anyway...

    I think the strategy should be: we accept the moving back to C++14 so master is fully functional, and we open an issue, whereby we should make sure there are no ROOT includes in any nvcc-compiled files.

    Alternatively, we could just stick to C++14 and move to C++17 whenever nvcc supports it.

    Edited by Daniel Hugo Campora Perez
  • Flo Reiss
    Flo Reiss @freiss ·
    Developer

    Seems sensible. From what I understood from @dovombru !105 (merged) also wants to move to C++17 and is introducing some changes to try to make it work, although there also seemed to be a problem when compiling with ROOT

  • Roel Aaij
    Roel Aaij @raaij ·
    Maintainer

    To build with the LHCb stack and ROOT, C++17 is required.

    Since CUDA is max C++14, some separation is needed, but this is not hard. If you look at !105 (merged) for the PV monitoring, you can see how that was done. Basically all ROOT code should be in host-only .cpp files and it can be called from the visitors.

  • Daniel Hugo Campora Perez
    Daniel Hugo Campora Perez @dcampora ·
    Maintainer

    @raaij could you please make a MR with those changes only? I will close this MR and accept that one.

  • Roel Aaij
    Roel Aaij @raaij ·
    Maintainer

    I'm not sure I can disentangle those easily from the rest of !105 (merged).

    Since the issue with ROOT is identical there (and I'm pretty sure can be fixed by building with the latest ROOT, because I can build that way), I suggest @dovombru and @freiss check if they can build branch dovombru_raaij_non_event_data_rebase_master with the latest ROOT, and if no new issues show up, !105 (merged) is merged. This MR can then be closed.

  • Roel Aaij
    Roel Aaij @raaij ·
    Maintainer

    I've rebased !105 (merged) and fixed the issues with ROOT there, I suggest to keep the discussions about this issue in one place, on !105 (merged).

  • Daniel Hugo Campora Perez
    Daniel Hugo Campora Perez @dcampora ·
    Maintainer

    I close this one, as we will keep C++17 in host code.

  • Daniel Hugo Campora Perez closed

    closed

Please register or sign in to reply