From 23a1223f2622410e3c00b7b0876fe07ab6f9e3ba Mon Sep 17 00:00:00 2001 From: gratienj Date: Wed, 19 Apr 2023 16:18:32 +0200 Subject: [PATCH] Fixe Solver Statistics management regression. (#187) * Fixe Solver Statistics management regression. Improve the management to reduce the impact on the API. Separation of Status and Stats structure from the Stater structure that computes and fill the Stat structure * clang format correction * fix clang-format * clang-format * fix export problem --- plugins/ginkgo/src/ginkgo_linear_solver.h | 1 + plugins/hypre/src/hypre_linear_solver.h | 2 + plugins/petsc/src/petsc_linear_solver.cpp | 2 + plugins/trilinos/src/trilinos_linear_solver.h | 4 +- .../core/backend/IInternalLinearSolverT.h | 10 + src/core/alien/core/backend/LinearSolver.h | 4 + src/core/alien/core/backend/LinearSolverT.h | 8 +- .../alien/expression/solver/SolverStat.cc | 12 ++ src/core/alien/expression/solver/SolverStat.h | 4 +- .../alien/expression/solver/SolverStater.cc | 122 +----------- .../alien/expression/solver/SolverStater.h | 173 +++++++++++++----- 11 files changed, 180 insertions(+), 162 deletions(-) diff --git a/plugins/ginkgo/src/ginkgo_linear_solver.h b/plugins/ginkgo/src/ginkgo_linear_solver.h index e5745148e..ca34176ec 100644 --- a/plugins/ginkgo/src/ginkgo_linear_solver.h +++ b/plugins/ginkgo/src/ginkgo_linear_solver.h @@ -77,6 +77,7 @@ class InternalLinearSolver //! Etat du solveur const SolverStatus& getStatus() const override; const SolverStat& getSolverStat() const override { return m_stat; } + SolverStat& getSolverStat() override { return m_stat; } std::shared_ptr algebra() const override; diff --git a/plugins/hypre/src/hypre_linear_solver.h b/plugins/hypre/src/hypre_linear_solver.h index 025f81a4e..3643fea6d 100644 --- a/plugins/hypre/src/hypre_linear_solver.h +++ b/plugins/hypre/src/hypre_linear_solver.h @@ -59,6 +59,8 @@ class InternalLinearSolver : public IInternalLinearSolver const SolverStat& getSolverStat() const final { return m_stat; } + SolverStat& getSolverStat() override { return m_stat; } + std::shared_ptr algebra() const final; private: diff --git a/plugins/petsc/src/petsc_linear_solver.cpp b/plugins/petsc/src/petsc_linear_solver.cpp index 073f6312f..594bbab47 100644 --- a/plugins/petsc/src/petsc_linear_solver.cpp +++ b/plugins/petsc/src/petsc_linear_solver.cpp @@ -68,6 +68,8 @@ class InternalLinearSolver const SolverStat& getSolverStat() const override { return m_stat; } + SolverStat& getSolverStat() override { return m_stat; } + std::shared_ptr algebra() const override; private: diff --git a/plugins/trilinos/src/trilinos_linear_solver.h b/plugins/trilinos/src/trilinos_linear_solver.h index 96c855e91..39d142a70 100644 --- a/plugins/trilinos/src/trilinos_linear_solver.h +++ b/plugins/trilinos/src/trilinos_linear_solver.h @@ -57,6 +57,8 @@ class InternalLinearSolver : public IInternalLinearSolver const SolverStat& getSolverStat() const { return m_stat; } + SolverStat& getSolverStat() override { return m_stat; } + std::shared_ptr algebra() const; private: @@ -72,4 +74,4 @@ class InternalLinearSolver : public IInternalLinearSolver void checkError(const Arccore::String& msg, int ierr, int skipError = 0) const; }; -} // namespace Alien::Trilinos \ No newline at end of file +} // namespace Alien::Trilinos diff --git a/src/core/alien/core/backend/IInternalLinearSolverT.h b/src/core/alien/core/backend/IInternalLinearSolverT.h index eb64dfbaf..985e14789 100644 --- a/src/core/alien/core/backend/IInternalLinearSolverT.h +++ b/src/core/alien/core/backend/IInternalLinearSolverT.h @@ -97,6 +97,16 @@ class IInternalLinearSolver */ virtual SolverStat const& getSolverStat() const = 0; + /*! + * \brief Get statistics on the solve phase + * + * Get statistics on the solver phase, such as iteration count, initialization time, + * solve time, etc. + * + * \return Solver statistics + */ + virtual SolverStat& getSolverStat() = 0; + /*! * \brief Indicates if the kernel is parallel * \returns Parallel support capability diff --git a/src/core/alien/core/backend/LinearSolver.h b/src/core/alien/core/backend/LinearSolver.h index 518cf8d92..1cc78919a 100644 --- a/src/core/alien/core/backend/LinearSolver.h +++ b/src/core/alien/core/backend/LinearSolver.h @@ -34,6 +34,7 @@ #include +#include /*---------------------------------------------------------------------------*/ /*---------------------------------------------------------------------------*/ @@ -77,6 +78,7 @@ class LinearSolver : public ILinearSolver template LinearSolver(T... args) : m_solver(AlgebraTraits::solver_factory(args...)) + , m_stater(m_solver.get()) {} //! Free resources @@ -102,6 +104,7 @@ class LinearSolver : public ILinearSolver * \param[in] pm : new parallel mng */ void updateParallelMng(Arccore::MessagePassing::IMessagePassingMng* pm); + /*! * \brief Solve the linear system A * x = b * \param[in] A The matrix to invert @@ -162,6 +165,7 @@ class LinearSolver : public ILinearSolver private: //! The linear solver kernel std::unique_ptr m_solver; + SolverStater m_stater; }; /*---------------------------------------------------------------------------*/ diff --git a/src/core/alien/core/backend/LinearSolverT.h b/src/core/alien/core/backend/LinearSolverT.h index 8a7c65650..52b9906f2 100644 --- a/src/core/alien/core/backend/LinearSolverT.h +++ b/src/core/alien/core/backend/LinearSolverT.h @@ -75,11 +75,14 @@ bool LinearSolver::solve(const IMatrix& A, const IVector& b, IVector& x) else { m_solver->updateParallelMng(A.impl()->distribution().parallelMng()); } - // m_solver->getSolverStat().startPrepareMeasure(); + + SolverStatSentry sentry(m_stater, BaseSolverStater::ePrepare); const auto& matrix = A.impl()->get(); const auto& rhs = b.impl()->get(); auto& sol = x.impl()->get(true); - // m_solver->getSolverStat().stopPrepareMeasure(); + sentry.release(); + + SolverStatSentry sentry2(m_stater, BaseSolverStater::eSolve); return m_solver->solve(matrix, rhs, sol); } @@ -89,6 +92,7 @@ bool LinearSolver::solve(const IMatrix& A, const IVector& b, IVector& x) template void LinearSolver::init() { + SolverStatSentry sentry(m_stater, BaseSolverStater::eInit); m_solver->init(); } diff --git a/src/core/alien/expression/solver/SolverStat.cc b/src/core/alien/expression/solver/SolverStat.cc index 66fed2a76..19b1139c8 100644 --- a/src/core/alien/expression/solver/SolverStat.cc +++ b/src/core/alien/expression/solver/SolverStat.cc @@ -32,6 +32,7 @@ namespace Alien { using namespace Arccore; + /*---------------------------------------------------------------------------*/ /*---------------------------------------------------------------------------*/ @@ -81,6 +82,17 @@ SolverStat::SolverStat() {} /*---------------------------------------------------------------------------*/ +void SolverStat::reset() +{ + m_solve_count = 0; + m_iteration_count = 0; + m_last_iteration_count = 0; + m_initialization_time = m_initialization_cpu_time = 0; + m_prepare_time = m_prepare_cpu_time = 0; + m_last_prepare_time = m_last_prepare_cpu_time = 0; + m_solve_time = m_solve_cpu_time = 0; + m_last_solve_time = m_last_solve_cpu_time = 0; +} Integer SolverStat::solveCount() const diff --git a/src/core/alien/expression/solver/SolverStat.h b/src/core/alien/expression/solver/SolverStat.h index 06cb12d46..b6f48d034 100644 --- a/src/core/alien/expression/solver/SolverStat.h +++ b/src/core/alien/expression/solver/SolverStat.h @@ -35,6 +35,7 @@ namespace Alien class ALIEN_EXPORT SolverStat { public: + template friend class SolverStater; /** Constructeur de la classe */ SolverStat(); @@ -57,6 +58,8 @@ class ALIEN_EXPORT SolverStat Real lastSolveTime() const; Real lastSolveCpuTime() const; + void reset(); + public: void print( ITraceMng* traceMng, const SolverStatus& status, String title = String()) const; @@ -78,7 +81,6 @@ class ALIEN_EXPORT SolverStat }; /*---------------------------------------------------------------------------*/ - } // namespace Alien /*---------------------------------------------------------------------------*/ diff --git a/src/core/alien/expression/solver/SolverStater.cc b/src/core/alien/expression/solver/SolverStater.cc index cb1133d81..91a1698a3 100644 --- a/src/core/alien/expression/solver/SolverStater.cc +++ b/src/core/alien/expression/solver/SolverStater.cc @@ -45,121 +45,9 @@ static clock_t current_clock_value = 0; #endif /*---------------------------------------------------------------------------*/ - -SolverStater::SolverStater() -: m_state(eNone) -, m_suspend_count(0) -{} - -/*---------------------------------------------------------------------------*/ - -void SolverStater::reset() -{ - m_solve_count = 0; - m_iteration_count = 0; - m_last_iteration_count = 0; - m_initialization_time = m_initialization_cpu_time = 0; - m_prepare_time = m_prepare_cpu_time = 0; - m_last_prepare_time = m_last_prepare_cpu_time = 0; - m_solve_time = m_solve_cpu_time = 0; - m_last_solve_time = m_last_solve_cpu_time = 0; - - m_state = eNone; - m_suspend_count = 0; -} - -/*---------------------------------------------------------------------------*/ - -void SolverStater::startInitializationMeasure() -{ - ALIEN_ASSERT((m_state == eNone), ("Unexpected SolverStater state %d", m_state)); - _startTimer(); - m_state = eInit; -} - -/*---------------------------------------------------------------------------*/ - -void SolverStater::stopInitializationMeasure() -{ - ALIEN_ASSERT((m_state == eInit), ("Unexpected SolverStater state %d", m_state)); - _stopTimer(); - m_state = eNone; - m_initialization_time += m_real_time; - m_initialization_cpu_time += m_cpu_time; -} - -/*---------------------------------------------------------------------------*/ - -void SolverStater::startPrepareMeasure() -{ - ALIEN_ASSERT((m_state == eNone), ("Unexpected SolverStater state %d", m_state)); - _startTimer(); - m_state = ePrepare; -} - -/*---------------------------------------------------------------------------*/ - -void SolverStater::suspendPrepareMeasure() -{ - ALIEN_ASSERT((m_state == ePrepare), ("Unexpected SolverStater state %d", m_state)); - _stopTimer(); - if (m_suspend_count == 0) { - m_last_prepare_time = m_real_time; - m_last_prepare_cpu_time = m_cpu_time; - } - else { - m_last_prepare_time += m_real_time; - m_last_prepare_cpu_time += m_cpu_time; - } - m_state = eNone; - ++m_suspend_count; -} - -/*---------------------------------------------------------------------------*/ - -void SolverStater::stopPrepareMeasure() -{ - if (m_state == ePrepare) - suspendPrepareMeasure(); - ALIEN_ASSERT((m_suspend_count > 0), ("Unexpected suspend count")); - - m_last_prepare_time += m_real_time; - m_last_prepare_cpu_time += m_cpu_time; - - m_suspend_count = 0; - m_state = eNone; - m_prepare_time += m_last_prepare_time; - m_prepare_cpu_time += m_last_prepare_cpu_time; -} - -/*---------------------------------------------------------------------------*/ - -void SolverStater::startSolveMeasure() -{ - ALIEN_ASSERT((m_state == eNone), ("Unexpected SolverStater state %d", m_state)); - _startTimer(); - m_state = eSolve; -} - -/*---------------------------------------------------------------------------*/ - -void SolverStater::stopSolveMeasure(const Alien::SolverStatus& status) -{ - ALIEN_ASSERT((m_state == eSolve), ("Unexpected SolverStater state %d", m_state)); - _stopTimer(); - m_state = eNone; - m_last_solve_time = m_real_time; - m_last_solve_cpu_time = m_cpu_time; - m_solve_time += m_last_solve_time; - m_solve_cpu_time += m_last_solve_cpu_time; - ++m_solve_count; - m_last_iteration_count = status.iteration_count; - m_iteration_count += m_last_iteration_count; -} - /*---------------------------------------------------------------------------*/ -Real SolverStater::_getVirtualTime() +Real BaseSolverStater::_getVirtualTime() { // From Arcane 1.16.3 to work with historical timers and Windows #ifdef ARCANE_TIMER_USE_CLOCK @@ -178,7 +66,7 @@ Real SolverStater::_getVirtualTime() /*---------------------------------------------------------------------------*/ -Real SolverStater::_getRealTime() +Real BaseSolverStater::_getRealTime() { // From Arcane 1.16.3 to work with old timers and Windows. #ifdef WIN32 @@ -202,7 +90,7 @@ Real SolverStater::_getRealTime() /*---------------------------------------------------------------------------*/ -void SolverStater::_errorInTimer(const String& msg, int retcode) +void BaseSolverStater::_errorInTimer(const String& msg, int retcode) { throw FatalErrorException( A_FUNCINFO, String::format("{0} return code: {1} errno: {2}", msg, retcode, errno)); @@ -210,7 +98,7 @@ void SolverStater::_errorInTimer(const String& msg, int retcode) /*---------------------------------------------------------------------------*/ -void SolverStater::_startTimer() +void BaseSolverStater::_startTimer() { ALIEN_ASSERT((m_state == eNone), ("Unexpected SolverStater state %d", m_state)); m_real_time = _getRealTime(); @@ -219,7 +107,7 @@ void SolverStater::_startTimer() /*---------------------------------------------------------------------------*/ -void SolverStater::_stopTimer() +void BaseSolverStater::_stopTimer() { ALIEN_ASSERT((m_state != eNone), ("Unexpected SolverStater state %d", m_state)); m_real_time = _getRealTime() - m_real_time; diff --git a/src/core/alien/expression/solver/SolverStater.h b/src/core/alien/expression/solver/SolverStater.h index 6f265e56e..b40add43a 100644 --- a/src/core/alien/expression/solver/SolverStater.h +++ b/src/core/alien/expression/solver/SolverStater.h @@ -28,8 +28,7 @@ namespace Alien { /*---------------------------------------------------------------------------*/ - -class ALIEN_EXPORT SolverStater : public SolverStat +class ALIEN_EXPORT BaseSolverStater { public: typedef enum @@ -41,28 +40,14 @@ class ALIEN_EXPORT SolverStater : public SolverStat } eStateType; public: - /** Constructeur de la classe */ - SolverStater(); + BaseSolverStater() + : m_state(eNone) + , m_suspend_count(0) + {} - /** Destructeur de la classe */ - virtual ~SolverStater() {} + virtual ~BaseSolverStater() {} public: - void reset(); - - void startInitializationMeasure(); - - void stopInitializationMeasure(); - - void startPrepareMeasure(); - - void suspendPrepareMeasure(); //!< Incremental contribution for prepare phase. - void stopPrepareMeasure(); - - void startSolveMeasure(); - - void stopSolveMeasure(const Alien::SolverStatus& status); - static Real getVirtualTimeCounter() { return _getVirtualTime(); } static Real getRealTimeCounter() { return _getRealTime(); } @@ -74,13 +59,14 @@ class ALIEN_EXPORT SolverStater : public SolverStat : m_counter(time_counter) , m_is_virtual(is_virtual) { - m_start_counter = m_is_virtual ? SolverStater::getVirtualTimeCounter() - : SolverStater::getRealTimeCounter(); + m_start_counter = m_is_virtual ? BaseSolverStater::getVirtualTimeCounter() + : BaseSolverStater::getRealTimeCounter(); } + virtual ~Sentry() { - Real end_counter = m_is_virtual ? SolverStater::getVirtualTimeCounter() - : SolverStater::getRealTimeCounter(); + Real end_counter = m_is_virtual ? BaseSolverStater::getVirtualTimeCounter() + : BaseSolverStater::getRealTimeCounter(); m_counter += end_counter - m_start_counter; } @@ -90,7 +76,7 @@ class ALIEN_EXPORT SolverStater : public SolverStat bool m_is_virtual; }; - private: + protected: static Arccore::Real _getVirtualTime(); static Arccore::Real _getRealTime(); @@ -101,39 +87,144 @@ class ALIEN_EXPORT SolverStater : public SolverStat void _stopTimer(); - private: + protected: eStateType m_state; Integer m_suspend_count; Real m_real_time; //!< 'wall clock' time for the lastest start or stop Real m_cpu_time; //!< 'cpu' time for the lastest start or stop }; -/*---------------------------------------------------------------------------*/ +template +class SolverStater : public BaseSolverStater +{ + public: + public: + /** Constructeur de la classe */ + SolverStater(SolverT* solver) + : BaseSolverStater() + , m_solver(solver) + {} + + /** Destructeur de la classe */ + virtual ~SolverStater() {} + + public: + void reset() + { + m_solver->getSolverStat().reset(); + + m_state = eNone; + m_suspend_count = 0; + } + void startInitializationMeasure() + { + ALIEN_ASSERT((m_state == eNone), ("Unexpected SolverStater state %d", m_state)); + _startTimer(); + m_state = eInit; + } + + void stopInitializationMeasure() + { + ALIEN_ASSERT((m_state == eInit), ("Unexpected SolverStater state %d", m_state)); + _stopTimer(); + m_state = eNone; + + auto& solver_stat = m_solver->getSolverStat(); + solver_stat.m_initialization_time += m_real_time; + solver_stat.m_initialization_cpu_time += m_cpu_time; + } + + void startPrepareMeasure() + { + ALIEN_ASSERT((m_state == eNone), ("Unexpected SolverStater state %d", m_state)); + _startTimer(); + m_state = ePrepare; + } + + void suspendPrepareMeasure() //!< Incremental contribution for prepare phase. + { + ALIEN_ASSERT((m_state == ePrepare), ("Unexpected SolverStater state %d", m_state)); + _stopTimer(); + auto& solver_stat = m_solver->getSolverStat(); + if (m_suspend_count == 0) { + solver_stat.m_last_prepare_time = m_real_time; + solver_stat.m_last_prepare_cpu_time = m_cpu_time; + } + else { + solver_stat.m_last_prepare_time += m_real_time; + solver_stat.m_last_prepare_cpu_time += m_cpu_time; + } + m_state = eNone; + ++m_suspend_count; + } + + void stopPrepareMeasure() + { + if (m_state == ePrepare) + suspendPrepareMeasure(); + ALIEN_ASSERT((m_suspend_count > 0), ("Unexpected suspend count")); + + auto& solver_stat = m_solver->getSolverStat(); + solver_stat.m_last_prepare_time += m_real_time; + solver_stat.m_last_prepare_cpu_time += m_cpu_time; + + m_suspend_count = 0; + m_state = eNone; + solver_stat.m_prepare_time += solver_stat.m_last_prepare_time; + solver_stat.m_prepare_cpu_time += solver_stat.m_last_prepare_cpu_time; + } + + void startSolveMeasure() + { + ALIEN_ASSERT((m_state == eNone), ("Unexpected SolverStater state %d", m_state)); + _startTimer(); + m_state = eSolve; + } + + void stopSolveMeasure() + { + ALIEN_ASSERT((m_state == eSolve), ("Unexpected SolverStater state %d", m_state)); + _stopTimer(); + m_state = eNone; + auto const& status = m_solver->getStatus(); + auto& solver_stat = m_solver->getSolverStat(); + solver_stat.m_last_solve_time = m_real_time; + solver_stat.m_last_solve_cpu_time = m_cpu_time; + solver_stat.m_solve_time += solver_stat.m_last_solve_time; + solver_stat.m_solve_cpu_time += solver_stat.m_last_solve_cpu_time; + ++solver_stat.m_solve_count; + solver_stat.m_last_iteration_count = status.iteration_count; + solver_stat.m_iteration_count += solver_stat.m_last_iteration_count; + } + + private: + SolverT* m_solver = nullptr; +}; + +/*---------------------------------------------------------------------------*/ template class SolverStatSentry { private: bool m_is_released = false; - Alien::SolverStatus& m_solver_status; - SolverStater& m_solver_stater; - SolverStater::eStateType m_state = SolverStater::eNone; + SolverStater m_solver_stater; + BaseSolverStater::eStateType m_state = BaseSolverStater::eNone; public: - SolverStatSentry(SolverT* solver, SolverStater::eStateType state) - : m_solver_status(solver->getStatusRef()) - , m_solver_stater(solver->getSolverStater()) + SolverStatSentry(SolverStater& parent, BaseSolverStater::eStateType state) + : m_solver_stater(parent) , m_state(state) { switch (m_state) { - case SolverStater::eInit: + case BaseSolverStater::eInit: m_solver_stater.reset(); m_solver_stater.startInitializationMeasure(); break; - case SolverStater::ePrepare: + case BaseSolverStater::ePrepare: m_solver_stater.startPrepareMeasure(); break; - case SolverStater::eSolve: + case BaseSolverStater::eSolve: m_solver_stater.startSolveMeasure(); break; default: @@ -148,14 +239,14 @@ class SolverStatSentry if (m_is_released) return; switch (m_state) { - case SolverStater::eInit: + case BaseSolverStater::eInit: m_solver_stater.stopInitializationMeasure(); break; - case SolverStater::ePrepare: + case BaseSolverStater::ePrepare: m_solver_stater.stopPrepareMeasure(); break; - case SolverStater::eSolve: - m_solver_stater.stopSolveMeasure(m_solver_status); + case BaseSolverStater::eSolve: + m_solver_stater.stopSolveMeasure(); break; default: break;