From eddb70e607afbb3e6cba0bbdbe9f1d0a24135057 Mon Sep 17 00:00:00 2001 From: Wouter Deconinck Date: Mon, 14 Aug 2023 13:22:03 -0500 Subject: [PATCH 1/3] feat: streamline utils with upstream edm4hep --- utils/CMakeLists.txt | 10 ++--- .../edm4eic/{ => utils}/analysis_utils.h | 0 utils/include/edm4eic/utils/dataframe.h | 15 ++++++++ utils/include/edm4eic/utils/kinematics.h | 38 +++++++++++++++++++ .../include/edm4eic/{ => utils}/unit_system.h | 0 .../edm4eic/{ => utils}/vector_utils.h | 0 .../edm4eic/{ => utils}/vector_utils_legacy.h | 0 7 files changed, 58 insertions(+), 5 deletions(-) rename utils/include/edm4eic/{ => utils}/analysis_utils.h (100%) create mode 100644 utils/include/edm4eic/utils/dataframe.h create mode 100644 utils/include/edm4eic/utils/kinematics.h rename utils/include/edm4eic/{ => utils}/unit_system.h (100%) rename utils/include/edm4eic/{ => utils}/vector_utils.h (100%) rename utils/include/edm4eic/{ => utils}/vector_utils_legacy.h (100%) diff --git a/utils/CMakeLists.txt b/utils/CMakeLists.txt index 438bfc7..86952f9 100644 --- a/utils/CMakeLists.txt +++ b/utils/CMakeLists.txt @@ -19,11 +19,11 @@ target_include_directories(edm4eic_utils ) install(FILES - include/edm4eic/analysis_utils.h - include/edm4eic/unit_system.h - include/edm4eic/vector_utils.h - include/edm4eic/vector_utils_legacy.h - DESTINATION include/edm4eic + include/edm4eic/utils/analysis_utils.h + include/edm4eic/utils/unit_system.h + include/edm4eic/utils/vector_utils.h + include/edm4eic/utils/vector_utils_legacy.h + DESTINATION include/edm4eic/utils ) install(TARGETS edm4eic_utils diff --git a/utils/include/edm4eic/analysis_utils.h b/utils/include/edm4eic/utils/analysis_utils.h similarity index 100% rename from utils/include/edm4eic/analysis_utils.h rename to utils/include/edm4eic/utils/analysis_utils.h diff --git a/utils/include/edm4eic/utils/dataframe.h b/utils/include/edm4eic/utils/dataframe.h new file mode 100644 index 0000000..1ff9d38 --- /dev/null +++ b/utils/include/edm4eic/utils/dataframe.h @@ -0,0 +1,15 @@ +#ifndef EDM4EIC_UTILS_DATAFRAME_H +#define EDM4EIC_UTILS_DATAFRAME_H + +#include + +namespace edm4eic::utils { + + using edm4hep::utils::pt; + using edm4hep::utils::eta; + using edm4hep::utils::cos_theta; + using edm4hep::utils::r; + +} // namespace edm4eic::utils + +#endif // EDM4EIC_UTILS_DATAFRAME_H diff --git a/utils/include/edm4eic/utils/kinematics.h b/utils/include/edm4eic/utils/kinematics.h new file mode 100644 index 0000000..0252ff7 --- /dev/null +++ b/utils/include/edm4eic/utils/kinematics.h @@ -0,0 +1,38 @@ +#ifndef EDM4EIC_UTILS_KINEMATICS_H +#define EDM4EIC_UTILS_KINEMATICS_H + +#include + +namespace edm4eic { +/** + * A LorentzVector with (px, py, pz) and M + */ +using edm4hep::LorentzVectorM; + +/** + * A LorentzVector with (px, py, pz) and E + */ +using edm4hep::LorentzVectorE; + +namespace utils { + + /** + * Get the transverse momentum from a Particle + */ + using edm4hep::utils::pT; + using edm4hep::utils::pt; + + /** + * Get the total momentum from a Particle + */ + using edm4hep::utils::p; + + /** + * Get the 4 momentum vector from a Particle using the momentum and the mass. + */ + using edm4hep::utils::p4; + +} // namespace utils +} // namespace edm4eic + +#endif diff --git a/utils/include/edm4eic/unit_system.h b/utils/include/edm4eic/utils/unit_system.h similarity index 100% rename from utils/include/edm4eic/unit_system.h rename to utils/include/edm4eic/utils/unit_system.h diff --git a/utils/include/edm4eic/vector_utils.h b/utils/include/edm4eic/utils/vector_utils.h similarity index 100% rename from utils/include/edm4eic/vector_utils.h rename to utils/include/edm4eic/utils/vector_utils.h diff --git a/utils/include/edm4eic/vector_utils_legacy.h b/utils/include/edm4eic/utils/vector_utils_legacy.h similarity index 100% rename from utils/include/edm4eic/vector_utils_legacy.h rename to utils/include/edm4eic/utils/vector_utils_legacy.h From 9f74f313be8f8a4060a3d204d90a5a48e74e77ec Mon Sep 17 00:00:00 2001 From: Wouter Deconinck Date: Mon, 14 Aug 2023 13:50:49 -0500 Subject: [PATCH 2/3] fix: support unit testing This fixes an earlier premature commit directly into main. --- test/CMakeLists.txt | 35 +++------- test/utils/CMakeLists.txt | 32 +++++++++ test/utils/test_kinematics.cpp | 121 +++++++++++++++++++++++++++++++++ test/utils/test_kinematics.py | 63 +++++++++++++++++ utils/CMakeLists.txt | 9 +-- 5 files changed, 231 insertions(+), 29 deletions(-) create mode 100644 test/utils/CMakeLists.txt create mode 100644 test/utils/test_kinematics.cpp create mode 100644 test/utils/test_kinematics.py diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 4d39a84..ecd08e3 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -6,8 +6,8 @@ ENDIF() function(set_test_env _testname) set_property(TEST ${_testname} APPEND PROPERTY ENVIRONMENT - LD_LIBRARY_PATH=$:$:$<$:$:>$:$ENV{LD_LIBRARY_PATH} - ROOT_INCLUDE_PATH=${PROJECT_SOURCE_DIR}/edm4hep:${PROJECT_SOURCE_DIR}/utils/include:$ENV{ROOT_INCLUDE_PATH} + LD_LIBRARY_PATH=$:$:$<$:$:>$:$ENV{LD_LIBRARY_PATH} + ROOT_INCLUDE_PATH=${PROJECT_SOURCE_DIR}/edm4eic:${PROJECT_SOURCE_DIR}/utils/include:$ENV{ROOT_INCLUDE_PATH} ) set_tests_properties(${_testname} PROPERTIES FAIL_REGULAR_EXPRESSION "[^a-z]Error;ERROR;error;Failed" @@ -15,14 +15,14 @@ function(set_test_env _testname) endfunction() add_executable(write_events write_events.cc) -target_include_directories(write_events PUBLIC ${PROJECT_SOURCE_DIR}/edm4hep ) -target_link_libraries(write_events edm4hep podio::podioRootIO) +target_include_directories(write_events PUBLIC ${PROJECT_SOURCE_DIR}/edm4eic ) +target_link_libraries(write_events edm4eic podio::podioRootIO) add_test(NAME write_events COMMAND write_events) set_test_env(write_events) add_executable(read_events read_events.cc) -target_include_directories(read_events PUBLIC ${PROJECT_SOURCE_DIR}/edm4hep ) -target_link_libraries(read_events edm4hep podio::podioRootIO) +target_include_directories(read_events PUBLIC ${PROJECT_SOURCE_DIR}/edm4eic ) +target_link_libraries(read_events edm4eic podio::podioRootIO) add_test(NAME read_events COMMAND read_events) set_property(TEST read_events PROPERTY DEPENDS write_events @@ -31,8 +31,8 @@ set_test_env(read_events) IF(TARGET ROOT::ROOTDataFrame) add_executable(test_rdf test_rdf.cc) - target_include_directories(test_rdf PUBLIC ${PROJECT_SOURCE_DIR}/edm4hep ${PROJECT_SOURCE_DIR}/dataframe ) - target_link_libraries(test_rdf edm4hepRDF ROOT::ROOTDataFrame podio::podioRootIO) + target_include_directories(test_rdf PUBLIC ${PROJECT_SOURCE_DIR}/edm4eic ${PROJECT_SOURCE_DIR}/dataframe ) + target_link_libraries(test_rdf edm4eicRDF ROOT::ROOTDataFrame podio::podioRootIO) add_test(NAME test_rdf COMMAND test_rdf) set_test_env(test_rdf) set_property(TEST test_rdf PROPERTY @@ -49,25 +49,10 @@ IF(TARGET ROOT::ROOTDataFrame) set_tests_properties(py_test_rdf PROPERTIES DEPENDS write_events) endif() - -find_package(HepMC3) -find_package(HepPDT) - -if(HepMC3_FOUND AND HepPDT_FOUND ) - add_executable(edm4hep_testhepmc hepmc/edm4hep_testhepmc.cc) - target_include_directories(edm4hep_testhepmc PUBLIC ${HEPMC3_INCLUDE_DIR} ${HEPPDT_INCLUDE_DIR} ) - target_link_libraries(edm4hep_testhepmc edm4hep podio::podioRootIO ${HEPPDT_LIBRARIES} ${HEPMC3_LIBRARIES}) - add_test(NAME edm4hep_testhepmc COMMAND edm4hep_testhepmc) - set_test_env(edm4hep_testhepmc) - set_property(TEST edm4hep_testhepmc APPEND PROPERTY ENVIRONMENT - ROOT_INCLUDE_PATH=${HEPMC3_INCLUDE_DIR}:$ENV{ROOT_INCLUDE_PATH} - ) -endif() - if (nlohmann_json_FOUND) - add_test(NAME convert_events COMMAND edm4hep2json edm4hep_events.root) + add_test(NAME convert_events COMMAND edm4hep2json edm4eic_events.root) set_property(TEST convert_events PROPERTY DEPENDS write_events) set_test_env(convert_events) endif() -add_subdirectory(utils) \ No newline at end of file +add_subdirectory(utils) diff --git a/test/utils/CMakeLists.txt b/test/utils/CMakeLists.txt new file mode 100644 index 0000000..e0ba9f8 --- /dev/null +++ b/test/utils/CMakeLists.txt @@ -0,0 +1,32 @@ +# SPDX-License-Identifier: Apache-2.0 + +if(USE_EXTERNAL_CATCH2) + find_package(Catch2 REQUIRED) +else() + message(STATUS "Fetching local copy of Catch2 library for unit-tests...") + + include(FetchContent) + FetchContent_Declare( + Catch2 + GIT_REPOSITORY https://github.com/catchorg/Catch2.git + GIT_TAG 6f21a3609cea360846a0ca93be55877cca14c86d + ) + FetchContent_MakeAvailable(Catch2) + set(CMAKE_MODULE_PATH ${Catch2_SOURCE_DIR}/extras ${CMAKE_MODULE_PATH}) +endif() + +include(Catch) + +add_executable(unittests + test_kinematics.cpp +) +target_link_libraries(unittests edm4eic EDM4EIC::utils Catch2::Catch2 Catch2::Catch2WithMain) +catch_discover_tests(unittests) + +add_test(NAME pyunittests COMMAND python -m unittest discover -s ${CMAKE_CURRENT_LIST_DIR}) +set_property(TEST pyunittests + PROPERTY ENVIRONMENT + LD_LIBRARY_PATH=$:$:$<$:$:>$:$ENV{LD_LIBRARY_PATH} + PYTHONPATH=${PROJECT_SOURCE_DIR}/python:$ENV{PYTHONPATH} + ROOT_INCLUDE_PATH=${PROJECT_SOURCE_DIR}/edm4eic:${PROJECT_SOURCE_DIR}/utils/include:$ENV{ROOT_INCLUDE_PATH} + ) diff --git a/test/utils/test_kinematics.cpp b/test/utils/test_kinematics.cpp new file mode 100644 index 0000000..66f6690 --- /dev/null +++ b/test/utils/test_kinematics.cpp @@ -0,0 +1,121 @@ +// SPDX-License-Identifier: Apache-2.0 + +#define CATCH_CONFIG_FAST_COMPILE +#include +#include +#include + +#include "edm4hep/utils/kinematics.h" + +#include "edm4hep/MutableMCParticle.h" +#include "edm4hep/MutableReconstructedParticle.h" + +#include "Math/Vector4D.h" + +#include + +using ParticleTypes = std::tuple; + +TEMPLATE_LIST_TEST_CASE("pT: transverse momentum", "[pT][kinematics]", ParticleTypes) { + using namespace edm4hep; + TestType particle; + + particle.setMomentum({3.0f, 4.0f, 0.0f}); + REQUIRE(utils::pT(particle) == 5.0f); + + particle.setMomentum({3.0f, 4.0f, 125.0f}); + REQUIRE(utils::pT(particle) == 5.0f); + + particle.setMomentum({-3.0f, 4.0f, 10.0f}); + REQUIRE(utils::pt(particle) == 5.0f); + + particle.setMomentum({-4.0f, -3.0f, std::nanf("")}); + REQUIRE(utils::pt(particle) == 5.0f); + + particle.setMomentum({std::nanf(""), -3.0f, 0}); + REQUIRE(std::isnan(utils::pt(particle))); +} + +TEMPLATE_LIST_TEST_CASE("p: momentum", "[p][kinematics]", ParticleTypes) { + using namespace edm4hep; + TestType particle; + + particle.setMomentum({1.0f, 2.0f, 3.0f}); + REQUIRE(utils::p(particle) == Catch::Approx(std::sqrt(14))); + + particle.setMomentum({1.0f, -2.0f, 3.0f}); + REQUIRE(utils::p(particle) == Catch::Approx(std::sqrt(14))); + + particle.setMomentum({1.0f, 2.0f, -3.0f}); + REQUIRE(utils::p(particle) == Catch::Approx(std::sqrt(14))); + + particle.setMomentum({-12.0f, 0.0f, 0.0f}); + REQUIRE(utils::p(particle) == 12.0f); + + particle.setMomentum({0.0f, -10.0f, -10.f}); + REQUIRE(utils::p(particle) == Catch::Approx(std::sqrt(200.0))); + + particle.setMomentum({std::nanf(""), 2.0f, 3.0f}); + REQUIRE(std::isnan(utils::p(particle))); +} + +TEST_CASE("p4: four momentum - MCParticle", "[p4][kinematics][MCParticle]") { + using namespace edm4hep; + MutableMCParticle particle; + particle.setMomentum({1.0f, 2.0f, 3.0f}); + particle.setMass(42); + REQUIRE(utils::p4(particle) == LorentzVectorM{1, 2, 3, 42}); + + // this basically just tests that the internal calculation of the energy works as expected + REQUIRE(utils::p4(particle, utils::UseEnergy) == LorentzVectorE{1, 2, 3, std::sqrt(14 + 42 * 42)}); +} + +TEST_CASE("p4: four momentum - ReconstructedParticle", "[p4][kinematics][ReconstructedParticle]") { + using namespace edm4hep; + MutableReconstructedParticle particle; + particle.setMomentum({1.0f, 2.0f, 3.0f}); + particle.setMass(125); + + // By default we use the mass + REQUIRE(utils::p4(particle) == LorentzVectorM{1, 2, 3, 125}); + + // In the case of ReconstructedParticle, the 4-momentum state is not kept + // consistent internally! So, when using the energy we will have a different 4 + // momentum vector + REQUIRE(utils::p4(particle, utils::UseEnergy) == LorentzVectorE{1, 2, 3, 0}); + + // Setting the Energy does not affect the mass + particle.setEnergy(42); + REQUIRE(utils::p4(particle, utils::UseMass) == LorentzVectorM{1, 2, 3, 125}); + REQUIRE(utils::p4(particle, utils::UseEnergy) == LorentzVectorE{1, 2, 3, 42}); +} + +TEST_CASE("p4 with user set values", "[p4][kinematics][user set values]") { + using namespace edm4hep; + MutableReconstructedParticle particle; + particle.setMomentum({1.0f, 2.0f, 3.0f}); + particle.setMass(125.0f); + particle.setEnergy(42.0f); + + // Requiring a dedicated mass value gives us a 4-vector with that mass value + REQUIRE(utils::p4(particle, utils::SetMass{3.096f}) == LorentzVectorM{1.0f, 2.0f, 3.0f, 3.096f}); + // The mass of the particle itself remains unchanged! + REQUIRE(particle.getMass() == 125.f); + + // Similar with the energy, if we want a dedicated value we get it in the 4 vector + REQUIRE(utils::p4(particle, utils::SetEnergy{1.23f}) == LorentzVectorE{1.0f, 2.0f, 3.0f, 1.23f}); + // But the underlying particle energy remains unchanged + REQUIRE(particle.getEnergy() == 42.f); + + // Make sure everything still works with the MC particle + MutableMCParticle mcPart; + mcPart.setMomentum({-1.0f, -2.0f, -3.0f}); + mcPart.setMass(125.0f); + + REQUIRE(utils::p4(mcPart, utils::SetMass{3.096f}) == LorentzVectorM{-1.0f, -2.0f, -3.0f, 3.096f}); + REQUIRE(mcPart.getMass() == 125.0f); // mass remains unchanged + + // everything needs to work with const classes as well + const auto mcPart2 = mcPart; + REQUIRE(utils::p4(mcPart2, utils::SetMass{3.096f}) == LorentzVectorM{-1.0f, -2.0f, -3.0f, 3.096f}); +} diff --git a/test/utils/test_kinematics.py b/test/utils/test_kinematics.py new file mode 100644 index 0000000..e12c79b --- /dev/null +++ b/test/utils/test_kinematics.py @@ -0,0 +1,63 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: Apache-2.0 + +import unittest +import cppyy + +import edm4hep + +# A few shorthands for slightly easier to read tests below +p4 = edm4hep.utils.p4 +LVM = edm4hep.LorentzVectorM +LVE = edm4hep.LorentzVectorE +SetEnergy = edm4hep.utils.SetEnergy +SetMass = edm4hep.utils.SetMass +UseEnergy = edm4hep.utils.UseEnergy +UseMass = edm4hep.utils.UseMass + + +class TestKinematics(unittest.TestCase): + """Tests to ensure that the kinematics header only library also works in + python""" + + def test_p4(self): + """Test the p4 functionality since that has shown to trip-up cppyy""" + # podio doesn't expose the Mutable constructors so we have to use the full + # glory of the immutable types here + p = edm4hep.MCParticle(11, # PDG + -1, # generatorStatus + -1, # simulatorStatus + 1.0, # charge + 0.0, # time + 125.0, # charge + edm4hep.Vector3d(0, 0, 0), # vertex + edm4hep.Vector3d(0, 0, 0), # endpoint + edm4hep.Vector3f(1.0, 2.0, 3.0), # momentum + edm4hep.Vector3f(0, 0, 0), # momentumAtEndpoint + edm4hep.Vector3f(0, 0, 0), # spin + edm4hep.Vector2i(0, 0) # colorFlow + ) + + self.assertEqual(p4(p), LVM(1.0, 2.0, 3.0, 125.0)) + self.assertEqual(p4(p, SetEnergy(42.0)), LVE(1.0, 2.0, 3.0, 42.0)) + self.assertEqual(p4(p, SetMass(250.0)), LVM(1.0, 2.0, 3.0, 250.0)) + + p = edm4hep.ReconstructedParticle(11, # type + 250.0, # energy + edm4hep.Vector3f(1.0, 2.0, 3.0), # momentum + edm4hep.Vector3f(0, 0, 0), # referencePoint + 1.0, # charge + 125.0, # mass + 0.0, # goodnessOfPID + cppyy.gbl.std.array('float', 10)() # covMatrix + ) + + self.assertEqual(p4(p), LVM(1.0, 2.0, 3.0, 125.0)) + self.assertEqual(p4(p, UseEnergy), LVE(1.0, 2.0, 3.0, 250.0)) + self.assertEqual(p4(p, UseMass), LVM(1.0, 2.0, 3.0, 125.0)) + self.assertEqual(p4(p, SetMass()), LVM(1.0, 2.0, 3.0, 0.0)) + self.assertEqual(p4(p, SetEnergy(42.0)), LVE(1.0, 2.0, 3.0, 42.0)) + + +if __name__ == '__main__': + unittest.main() diff --git a/utils/CMakeLists.txt b/utils/CMakeLists.txt index 86952f9..23344bf 100644 --- a/utils/CMakeLists.txt +++ b/utils/CMakeLists.txt @@ -4,15 +4,16 @@ find_package(ROOT REQUIRED COMPONENTS GenVector MathCore) find_package(CLI11 CONFIG) -add_library(edm4eic_utils INTERFACE) +add_library(utils INTERFACE) +add_library(EDM4EIC::utils ALIAS utils) -target_link_libraries(edm4eic_utils +target_link_libraries(utils INTERFACE edm4eic INTERFACE EDM4HEP::edm4hep INTERFACE ROOT::GenVector ROOT::MathCore ) -target_include_directories(edm4eic_utils +target_include_directories(utils INTERFACE $ INTERFACE $ INTERFACE $ @@ -26,7 +27,7 @@ install(FILES DESTINATION include/edm4eic/utils ) -install(TARGETS edm4eic_utils +install(TARGETS utils EXPORT ${PROJECT_NAME}Targets LIBRARY DESTINATION lib ARCHIVE DESTINATION lib From 8a77986aef6a7507f2b847308f9a5850b4b1ab5e Mon Sep 17 00:00:00 2001 From: Wouter Deconinck Date: Mon, 14 Aug 2023 13:52:51 -0500 Subject: [PATCH 3/3] ci: BUILD_TESTING=ON --- .github/workflows/linux-eic-shell.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/linux-eic-shell.yml b/.github/workflows/linux-eic-shell.yml index ca05dfa..927f105 100644 --- a/.github/workflows/linux-eic-shell.yml +++ b/.github/workflows/linux-eic-shell.yml @@ -21,7 +21,7 @@ jobs: platform-release: "jug_xl:nightly" run: | PREFIX=${PWD}/install - cmake -B build -S . -DCMAKE_INSTALL_PREFIX=${PREFIX} + cmake -B build -S . -DCMAKE_INSTALL_PREFIX=${PREFIX} -DBUILD_TESTING=ON cmake --build build -- install - uses: actions/upload-artifact@v3 with: