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.

Sign up for GitHub

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

Jump to bottom

Move to C++20 and remove vector_utils_legacy #343

Merged
merged 6 commits into from
Aug 27, 2024
Merged

Move to C++20 and remove vector_utils_legacy #343

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c2f77c5
Move to C++20 and get rid of vector_utils_legacy
jmcarcell Jul 21, 2024
80f8f2d
Remove the checks for consteval
jmcarcell Jul 23, 2024
ce3b7de
Update or remove workflows to use C++20
jmcarcell Aug 2, 2024
af3fba3
Use contains and std::ranges::find from C++20
jmcarcell Aug 5, 2024
dfe513f
Add back an LCG workflow and ubuntu
jmcarcell Aug 5, 2024
48a4ce9
Do not run on ubuntu since it doesn't have C++20
jmcarcell Aug 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 2 additions & 6 deletions .github/workflows/lcg_linux_with_podio.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,9 @@ jobs:
matrix:
LCG: ["dev3/x86_64-el9-clang16-opt",
"dev4/x86_64-el9-clang16-opt",
"dev4/x86_64-el9-gcc13-opt"]
"dev4/x86_64-el9-gcc13-opt",
"LCG_106/x86_64-el9-gcc13-opt"]
CXX_STANDARD: [20]
include:
- LCG: "LCG_106/x86_64-el9-gcc13-opt"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we keep one LCG release and an ubuntu build and just build them with c++20?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gcc13 is build with c++20 (so is clang16). Nothing on ubuntu yet.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think gcc11 on ubuntu22.04 should have enough c++20 capabilities for EDM4hep.

CXX_STANDARD: 17
- LCG: "dev4/x86_64-ubuntu2004-gcc9-opt"
CXX_STANDARD: 17
steps:
- uses: actions/checkout@v3
- uses: cvmfs-contrib/github-action-cvmfs@v3
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/pre-commit.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jobs:
cd podio
mkdir build install
cd build
cmake .. -DCMAKE_CXX_STANDARD=17 \
cmake .. -DCMAKE_CXX_STANDARD=20 \
-DENABLE_SIO=ON \
-DBUILD_TESTING=OFF \
-DCMAKE_INSTALL_PREFIX=$(pwd)/../install \
Expand All @@ -40,7 +40,7 @@ jobs:
mkdir build
cd build
cmake .. -DENABLE_SIO=ON \
-DCMAKE_CXX_STANDARD=17 \
-DCMAKE_CXX_STANDARD=20 \
-DCMAKE_CXX_FLAGS=" -fdiagnostics-color=always -Werror "\
-DCMAKE_EXPORT_COMPILE_COMMANDS=ON \
-DUSE_EXTERNAL_CATCH2=OFF
Expand Down
4 changes: 2 additions & 2 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@ if(NOT CMAKE_CONFIGURATION_TYPES)
endif()

# - Define the C++ Standard to use (Simplest Possible)
set(CMAKE_CXX_STANDARD 17 CACHE STRING "")
set(CMAKE_CXX_STANDARD 20 CACHE STRING "")

if(NOT CMAKE_CXX_STANDARD MATCHES "17|20")
if(NOT CMAKE_CXX_STANDARD MATCHES "20|23")
message(FATAL_ERROR "Unsupported C++ standard: ${CMAKE_CXX_STANDARD}")
endif()

Expand Down
6 changes: 3 additions & 3 deletions test/hepmc/edm4hep_testhepmc.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ int main() {
std::cout << "Converting particle with PDG ID: " << particle_i->pdg_id() << std::endl;
std::cout << "\t and id: " << particle_i->id() << std::endl;

if (hepmcToEdmMap.find(particle_i->id()) == hepmcToEdmMap.end()) {
if (!hepmcToEdmMap.contains(particle_i->id())) {
auto edm_particle = convert(particle_i);
hepmcToEdmMap.insert({particle_i->id(), edm_particle});
}
Expand All @@ -124,7 +124,7 @@ int main() {
if (nullptr != prodvertex) {

for (auto particle_mother : prodvertex->particles_in()) {
if (hepmcToEdmMap.find(particle_mother->id()) == hepmcToEdmMap.end()) {
if (!hepmcToEdmMap.contains(particle_mother->id())) {
auto edm_particle = convert(particle_mother);
hepmcToEdmMap.insert({particle_mother->id(), edm_particle});
}
Expand All @@ -136,7 +136,7 @@ int main() {
if (nullptr != prodvertex) {

for (auto particle_daughter : prodvertex->particles_in()) {
if (hepmcToEdmMap.find(particle_daughter->id()) == hepmcToEdmMap.end()) {
if (!hepmcToEdmMap.contains(particle_daughter->id())) {
auto edm_particle = convert(particle_daughter);
hepmcToEdmMap.insert({particle_daughter->id(), edm_particle});
}
Expand Down
4 changes: 0 additions & 4 deletions test/utils/test_covmatrix_utils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,6 @@ TEST_CASE("CovarianceMatrix indexing", "[cov_matrix_utils]") {

STATIC_REQUIRE(get_cov_dim(21) == 6);
STATIC_REQUIRE(get_cov_dim(1) == 1);
#if !__cpp_consteval
// This will fail to compile if we have consteval
REQUIRE_THROWS_AS(get_cov_dim(14), std::invalid_argument);
#endif

// clang-format off
// For better interpretability of the tests below, these are the indices of a
Expand Down
4 changes: 0 additions & 4 deletions utils/include/edm4hep/utils/cov_matrix_utils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,7 @@ namespace utils {
*
* @returns the dimension of the covariance matrix
*/
#if cpp_consteval
consteval std::size_t get_cov_dim(std::size_t N) {
#else
constexpr std::size_t get_cov_dim(std::size_t N) {
#endif
switch (N) {
case 21:
return 6;
Expand Down
7 changes: 0 additions & 7 deletions utils/include/edm4hep/utils/vector_utils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,6 @@
#ifndef EDM4HEP_UTILS_VECTOR_HH
#define EDM4HEP_UTILS_VECTOR_HH

// These ultilies require concepts. If not available, use the fallback
// vector_utils_legacy.h instead to capture most functionality.
#if !__cpp_concepts
#include <edm4hep/utils/vector_utils_legacy.h>
#else

#include <edm4hep/Vector3f.h>

#include <cmath>
Expand Down Expand Up @@ -327,4 +321,3 @@ inline constexpr V operator/(const V& v, const double d) {
} // namespace edm4hep

#endif
#endif
Loading
Loading