diff --git a/.cmake/testing.cmake b/.cmake/testing.cmake index 4db820a8d..4c310768f 100644 --- a/.cmake/testing.cmake +++ b/.cmake/testing.cmake @@ -1,21 +1,3 @@ -find_package(Git QUIET) - -if(Git_FOUND AND EXISTS "${PROJECT_SOURCE_DIR}/.git") - # Update submodules as needed - option(GIT_SUBMODULE "Check submodules during build" ON) - - if(GIT_SUBMODULE) - message(STATUS "Submodule update") - execute_process(COMMAND ${GIT_EXECUTABLE} submodule update --init --recursive - WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} - RESULT_VARIABLE GIT_SUBMOD_RESULT) - - if(NOT GIT_SUBMOD_RESULT EQUAL "0") - message(FATAL_ERROR "git submodule update --init failed with ${GIT_SUBMOD_RESULT}, please checkout submodules") - endif() - endif() -endif() - set(gtest_force_shared_crt ON CACHE BOOL "" FORCE) # Suppress project warning policy for googletest library @@ -80,4 +62,4 @@ if(${BUILD_WITH_GCOVR}) ) endif() -add_definitions(-DWITH_TESTS) \ No newline at end of file +add_definitions(-DWITH_TESTS) diff --git a/CMakeLists.txt b/CMakeLists.txt index 69212da33..8c6fd2078 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -165,6 +165,24 @@ endif() # ************************ set(CODE_INCLUDE_DIR ${PROJECT_SOURCE_DIR}/include) +find_package(Git QUIET) + +if(Git_FOUND AND EXISTS "${PROJECT_SOURCE_DIR}/.git") + # Update submodules as needed + option(GIT_SUBMODULE "Check submodules during build" ON) + + if(GIT_SUBMODULE) + message(STATUS "Submodule update") + execute_process(COMMAND ${GIT_EXECUTABLE} submodule update --init --recursive + WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} + RESULT_VARIABLE GIT_SUBMOD_RESULT) + + if(NOT GIT_SUBMOD_RESULT EQUAL "0") + message(FATAL_ERROR "git submodule update --init failed with ${GIT_SUBMOD_RESULT}, please checkout submodules") + endif() + endif() +endif() + include(eigen) if(BUILD_WITH_MPI) @@ -184,6 +202,9 @@ if(BUILD_WITH_TESTS) include(testing) endif() +add_library(mstd INTERFACE) +target_include_directories(mstd INTERFACE ${PROJECT_SOURCE_DIR}/external/mstd/include) + add_subdirectory(src) if(BUILD_WITH_TESTS) diff --git a/changes/developer/enhancement.timer.md b/changes/developer/enhancement.timer.md index 7ffe0ece7..cef43667a 100644 --- a/changes/developer/enhancement.timer.md +++ b/changes/developer/enhancement.timer.md @@ -1 +1,2 @@ - add guarded RAII approach for starting and automatically stopping a timings section with its destructor +- use a dedicated enum class for timers to make it less error prone diff --git a/external/mstd b/external/mstd index 5f933e7a5..768a13035 160000 --- a/external/mstd +++ b/external/mstd @@ -1 +1 @@ -Subproject commit 5f933e7a5202f63bbc2cd73c42f4d479998379d3 +Subproject commit 768a13035b7b12717547f881a18a21f91a15fb3b diff --git a/include/timings/timer.hpp b/include/timings/timer.hpp index bed809b7a..587c3902c 100644 --- a/include/timings/timer.hpp +++ b/include/timings/timer.hpp @@ -28,6 +28,7 @@ #include // for string #include // for vector +#include "timerId.hpp" #include "timingsSection.hpp" // for TimingsManager #include "timingsSectionGuard.hpp" @@ -48,12 +49,12 @@ namespace timings class Timer { protected: - std::string _name = "DefaultTimings"; + TimerId _id = TimerId::DefaultTimings; std::vector _timingDetails; public: - explicit Timer(const std::string_view); + explicit Timer(TimerId id); Timer() = default; [[nodiscard]] @@ -75,7 +76,7 @@ namespace timings * standard setters * ********************/ - void setTimerName(const std::string_view name); + void setTimerId(TimerId id); /******************** * standard getters * diff --git a/include/timings/timerId.hpp b/include/timings/timerId.hpp new file mode 100644 index 000000000..60ec5e07a --- /dev/null +++ b/include/timings/timerId.hpp @@ -0,0 +1,56 @@ +/***************************************************************************** + + + PQ + Copyright (C) 2023-now Jakob Gamper + + This program is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . + + +******************************************************************************/ + +#ifndef _TIMER_ID_HPP_ +#define _TIMER_ID_HPP_ + +#include +#include +#include + +#define TIMER_ID_LIST(X) \ + X(DefaultTimings) \ + X(EngineOutput) \ + X(Constraints) \ + X(CellList) \ + X(PhysicalData) \ + X(Simulation) \ + X(Integrator) \ + X(Thermostat) \ + X(Manostat) \ + X(Output) \ + X(Potential) \ + X(IntraNonBonded) \ + X(Virial) \ + X(ResetKinetics) \ + X(WaterIntraPotential) \ + X(WaterInterPotential) \ + X(QMEngine) \ + X(Setup) + +// TODO: fix this in mstd +// NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-constant-array-index) +MSTD_ENUM(TimerId, std::uint8_t, TIMER_ID_LIST); + +std::string toString(TimerId id); + +#endif // _TIMER_ID_HPP_ diff --git a/src/engine/hessianEngine.cpp b/src/engine/hessianEngine.cpp index a202d1252..a3c12f661 100644 --- a/src/engine/hessianEngine.cpp +++ b/src/engine/hessianEngine.cpp @@ -485,22 +485,22 @@ void HessianEngine::writeHessianInfo(const HessianMatrix &hessian) const void HessianEngine::addTimers() { - _engineOutput.setTimerName("Output"); + _engineOutput.setTimerId(TimerId::Output); _timer.addTimer(_engineOutput.getTimer()); - _constraints->setTimerName("Constraints"); + _constraints->setTimerId(TimerId::Constraints); _timer.addTimer(_constraints->getTimer()); - _cellList->setTimerName("Cell List"); + _cellList->setTimerId(TimerId::CellList); _timer.addTimer(_cellList->getTimer()); - _potential->setTimerName("Potential"); + _potential->setTimerId(TimerId::Potential); _timer.addTimer(_potential->getTimer()); - _intraNonBonded->setTimerName("IntraNonBonded"); + _intraNonBonded->setTimerId(TimerId::IntraNonBonded); _timer.addTimer(_intraNonBonded->getTimer()); - _physicalData->setTimerName("Physical Data"); + _physicalData->setTimerId(TimerId::PhysicalData); _timer.addTimer(_physicalData->getTimer()); } diff --git a/src/engine/mdEngine.cpp b/src/engine/mdEngine.cpp index d6e20e804..c808b1c90 100644 --- a/src/engine/mdEngine.cpp +++ b/src/engine/mdEngine.cpp @@ -77,49 +77,49 @@ void MDEngine::run() const auto elapsedTime = double(_timer.calculateElapsedTime()) * 1e-3; - _engineOutput.setTimerName("Output"); + _engineOutput.setTimerId(TimerId::Output); _timer.addTimer(_engineOutput.getTimer()); - _thermostat->setTimerName("Thermostat"); + _thermostat->setTimerId(TimerId::Thermostat); _timer.addTimer(_thermostat->getTimer()); - _integrator->setTimerName("Integrator"); + _integrator->setTimerId(TimerId::Integrator); _timer.addTimer(_integrator->getTimer()); - _constraints->setTimerName("Constraints"); + _constraints->setTimerId(TimerId::Constraints); _timer.addTimer(_constraints->getTimer()); - _cellList->setTimerName("Cell List"); + _cellList->setTimerId(TimerId::CellList); _timer.addTimer(_cellList->getTimer()); - _potential->setTimerName("Potential"); + _potential->setTimerId(TimerId::Potential); _timer.addTimer(_potential->getTimer()); - _intraNonBonded->setTimerName("IntraNonBonded"); + _intraNonBonded->setTimerId(TimerId::IntraNonBonded); _timer.addTimer(_intraNonBonded->getTimer()); - _virial->setTimerName("Virial"); + _virial->setTimerId(TimerId::Virial); _timer.addTimer(_virial->getTimer()); - _physicalData->setTimerName("Physical Data"); + _physicalData->setTimerId(TimerId::PhysicalData); _timer.addTimer(_physicalData->getTimer()); - _manostat->setTimerName("Manostat"); + _manostat->setTimerId(TimerId::Manostat); _timer.addTimer(_manostat->getTimer()); - _resetKinetics.setTimerName("Reset Kinetics"); + _resetKinetics.setTimerId(TimerId::ResetKinetics); _timer.addTimer(_resetKinetics.getTimer()); - _intraWater->setTimerName("Water Intra Potential"); + _intraWater->setTimerId(TimerId::WaterIntraPotential); _timer.addTimer(_intraWater->getTimer()); - _interWater->setTimerName("Water Inter Potential"); + _interWater->setTimerId(TimerId::WaterInterPotential); _timer.addTimer(_interWater->getTimer()); if (Settings::isQMActivated()) { - dynamic_cast(this)->getQMRunner()->setTimerName( - "QM Engine" + dynamic_cast(this)->getQMRunner()->setTimerId( + TimerId::QMEngine ); _timer.addTimer( dynamic_cast(this)->getQMRunner()->getTimer() diff --git a/src/engine/optEngine.cpp b/src/engine/optEngine.cpp index 25c9d8063..513db9219 100644 --- a/src/engine/optEngine.cpp +++ b/src/engine/optEngine.cpp @@ -91,22 +91,22 @@ void OptEngine::run() const auto elapsedTime = double(_timer.calculateElapsedTime()) * 1e-3; - _engineOutput.setTimerName("Output"); + _engineOutput.setTimerId(TimerId::Output); _timer.addTimer(_engineOutput.getTimer()); - _constraints->setTimerName("Constraints"); + _constraints->setTimerId(TimerId::Constraints); _timer.addTimer(_constraints->getTimer()); - _cellList->setTimerName("Cell List"); + _cellList->setTimerId(TimerId::CellList); _timer.addTimer(_cellList->getTimer()); - _potential->setTimerName("Potential"); + _potential->setTimerId(TimerId::Potential); _timer.addTimer(_potential->getTimer()); - _intraNonBonded->setTimerName("IntraNonBonded"); + _intraNonBonded->setTimerId(TimerId::IntraNonBonded); _timer.addTimer(_intraNonBonded->getTimer()); - _physicalData->setTimerName("Physical Data"); + _physicalData->setTimerId(TimerId::PhysicalData); _timer.addTimer(_physicalData->getTimer()); _engineOutput.writeTimingsFile(_timer); diff --git a/src/setup/setup.cpp b/src/setup/setup.cpp index b1a5e3a69..aa2f431be 100644 --- a/src/setup/setup.cpp +++ b/src/setup/setup.cpp @@ -74,7 +74,7 @@ using namespace setup::resetKinetics; */ void setup::setupRequestedJob(const std::string& inputFileName, Engine& engine) { - auto setupTimer = Timer("Setup"); + auto setupTimer = Timer(TimerId::Setup); auto _ = setupTimer.scoped("TotalSetup"); diff --git a/src/timings/CMakeLists.txt b/src/timings/CMakeLists.txt index 43ab23e42..8e7bedd82 100644 --- a/src/timings/CMakeLists.txt +++ b/src/timings/CMakeLists.txt @@ -1,5 +1,6 @@ add_library(timings timer.cpp + timerId.cpp timingsSection.cpp timingsSectionGuard.cpp globalTimer.cpp @@ -15,6 +16,7 @@ target_link_libraries(timings PUBLIC exceptions settings + mstd ) install(TARGETS timings diff --git a/src/timings/globalTimer.cpp b/src/timings/globalTimer.cpp index a90b8948d..101e3fe43 100644 --- a/src/timings/globalTimer.cpp +++ b/src/timings/globalTimer.cpp @@ -32,7 +32,7 @@ using namespace timings; * @brief Construct a new Global Timer:: Global Timer object * */ -GlobalTimer::GlobalTimer() : _simulationTimer("Simulation") +GlobalTimer::GlobalTimer() : _simulationTimer(TimerId::Simulation) { _simulationTimer.startTimingsSection(); } diff --git a/src/timings/timer.cpp b/src/timings/timer.cpp index e59f0d7cb..66275fa92 100644 --- a/src/timings/timer.cpp +++ b/src/timings/timer.cpp @@ -32,9 +32,9 @@ using namespace customException; /** * @brief Construct a new Timer:: Timer object * - * @param name + * @param id */ -Timer::Timer(const std::string_view name) : _name(name) {} +Timer::Timer(const TimerId id) : _id(id) {} /** * @brief get timings details @@ -79,11 +79,11 @@ double Timer::calculateLoopTime() const */ void Timer::startTimingsSection() { - const auto index = findTimingsSectionIndex(_name); + const auto index = findTimingsSectionIndex(getTimerName()); if (index == _timingDetails.size()) { - _timingDetails.emplace_back(_name); + _timingDetails.emplace_back(getTimerName()); _timingDetails.back().beginTimer(); } else @@ -113,7 +113,7 @@ void Timer::startTimingsSection(const std::string_view name) */ void Timer::stopTimingsSection() { - const auto index = findTimingsSectionIndex(_name); + const auto index = findTimingsSectionIndex(getTimerName()); if (index == _timingDetails.size()) throw CustomException("Timer not found"); @@ -166,11 +166,11 @@ void Timer::sortTimingsSections() ********************/ /** - * @brief set timer name + * @brief set timer id * - * @param name + * @param id */ -void Timer::setTimerName(const std::string_view name) { _name = name; } +void Timer::setTimerId(const TimerId id) { _id = id; } /******************** * standard getters * @@ -195,7 +195,7 @@ TimingsSection Timer::getTimingsSection(const std::string_view name) const * * @return std::string */ -std::string Timer::getTimerName() const { return _name; } +std::string Timer::getTimerName() const { return toString(_id); } /** * @brief get timer diff --git a/src/timings/timerId.cpp b/src/timings/timerId.cpp new file mode 100644 index 000000000..56006efac --- /dev/null +++ b/src/timings/timerId.cpp @@ -0,0 +1,55 @@ +/***************************************************************************** + + + PQ + Copyright (C) 2023-now Jakob Gamper + + This program is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . + + +******************************************************************************/ + +#include "timerId.hpp" + +/** + * @brief convert TimerId to string + * + * @param id + * @return std::string + */ +std::string toString(TimerId id) +{ + if (id == TimerId::DefaultTimings) + return "Default Timings"; + + if (id == TimerId::CellList) + return "Cell List"; + + if (id == TimerId::PhysicalData) + return "Physical Data"; + + if (id == TimerId::ResetKinetics) + return "Reset Kinetics"; + + if (id == TimerId::WaterIntraPotential) + return "Water Intra Potential"; + + if (id == TimerId::WaterInterPotential) + return "Water Inter Potential"; + + if (id == TimerId::QMEngine) + return "QM Engine"; + + return TimerIdMeta::toString(id); +} diff --git a/tests/src/output/testTimingsOutput.cpp b/tests/src/output/testTimingsOutput.cpp index b23b1b2de..dc996be14 100644 --- a/tests/src/output/testTimingsOutput.cpp +++ b/tests/src/output/testTimingsOutput.cpp @@ -81,7 +81,7 @@ TEST(TestTimingsOutput, writeListsRegisteredSubTimers) GlobalTimer global; global.startSimulationTimer(); - Timer t("MySection"); + Timer t(TimerId::DefaultTimings); { auto _ = t.scoped("inner"); std::this_thread::sleep_for(std::chrono::milliseconds(1)); @@ -95,7 +95,10 @@ TEST(TestTimingsOutput, writeListsRegisteredSubTimers) out.close(); const auto content = slurp(path); - EXPECT_NE(content.find("MySection"), std::string::npos); + EXPECT_NE( + content.find(toString(TimerId::DefaultTimings)), + std::string::npos + ); EXPECT_NE(content.find("inner"), std::string::npos); const auto errorCode = std::remove(path.c_str()); EXPECT_EQ(errorCode, 0) << "Failed to remove file: " << path;