fix the additional residual computation when passing residual criterion #1307
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MarcelKoch
approved these changes
Mar 22, 2023
There was a problem hiding this comment.
LGTM. I guess this could lead to infinite loops in user code, if they created their own solver, but used set_finalized incorrectly. But I think that is highly unlikely.
Alternatively, you could throw (some special error) in the if(!set_finalized) case and use a try-catch block in IR. But I think that might be too much overhead.
core/solver/gmres.cpp
Outdated
| @@ -302,7 +302,7 @@ void Gmres<ValueType>::apply_dense_impl(const VectorType* dense_b, | |||
| .residual(residual) | |||
| .residual_norm(residual_norm) | |||
| .solution(dense_x) | |||
| .check(RelativeStoppingId, false, &stop_status, &one_changed)) { | |||
| .check(RelativeStoppingId, true, &stop_status, &one_changed)) { | |||
There was a problem hiding this comment.
Any idea, why that was false in the first place?
There was a problem hiding this comment.
This was changed in #861 , since now the restart kernel takes care of the finalization status. We only have an accurate residual vector after a restart
There was a problem hiding this comment.
okay, so I think I can not change it here
core/stop/residual_norm.cpp
Outdated
| b_ != nullptr) { | ||
| } else if (set_finalized && updater.solution_ != nullptr && | ||
| system_matrix_ != nullptr && b_ != nullptr) { | ||
| // Only compute the residual from solution in the finalized step |
There was a problem hiding this comment.
nit:
Suggested change
| // Only compute the residual from solution in the finalized step | |
| // Only compute the residual from the solution in the finalized step |
core/stop/residual_norm.cpp
Outdated
| @@ -209,6 +210,10 @@ bool ResidualNormBase<ValueType>::check_impl( | |||
| }, | |||
| b_.get(), updater.solution_); | |||
| dense_tau = u_dense_tau_.get(); | |||
| } else if (!set_finalized) { | |||
| // If it is not the finalized step and does not contain residual, we | |||
There was a problem hiding this comment.
Suggested change
| // If it is not the finalized step and does not contain residual, we | |
| // If this is not the finalized step and the updater does not contain residual, we |
81c2633 to
e3d9aae
Compare
|
@MarcelKoch you are right, it may lead an infinite loop in some cases. I use another parameter to indicate whether we can skip the residual check |
490c2e1 to
81fab18
Compare
pratikvn
reviewed
Mar 23, 2023
upsj
approved these changes
Apr 6, 2023
ced2ab4 to
859b657
Compare
|
@upsj good idea. three iteration: |
Co-authored-by: Tobias Ribizel <ribizel@kit.edu> Co-authored-by: Pratik Nayak <pratikvn@protonmail.com>
859b657 to
23dc74a
Compare
|
I need to change stream logger to profilerhook summary because MSVC does not contain enough function infomation.
|
7067b39 to
3f03803
Compare
Co-authored-by: Pratik Nayak <pratikvn@protonmail.com>
MSVC does not contain enough function information.
3f03803 to
d986dbb
Compare
|
Note: This PR changes the Ginkgo ABI: For details check the full ABI diff under Artifacts here |
|
Kudos, SonarCloud Quality Gate passed! |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## develop #1307 +/- ##
========================================
Coverage 91.26% 91.27%
========================================
Files 584 584
Lines 49450 49483 +33
========================================
+ Hits 45129 45164 +35
+ Misses 4321 4319 -2
... and 4 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
tcojean
added a commit
that referenced
this pull request
Jun 16, 2023
Release 1.6.0 of Ginkgo. The Ginkgo team is proud to announce the new Ginkgo minor release 1.6.0. This release brings new features such as: - Several building blocks for GPU-resident sparse direct solvers like symbolic and numerical LU and Cholesky factorization, ..., - A distributed Schwarz preconditioner, - New FGMRES and GCR solvers, - Distributed benchmarks for the SpMV operation, solvers, ... - Support for non-default streams in the CUDA and HIP backends, - Mixed precision support for the CSR SpMV, - A new profiling logger which integrates with NVTX, ROCTX, TAU and VTune to provide internal Ginkgo knowledge to most HPC profilers! and much more. If you face an issue, please first check our [known issues page](https://github.com/ginkgo-project/ginkgo/wiki/Known-Issues) and the [open issues list](https://github.com/ginkgo-project/ginkgo/issues) and if you do not find a solution, feel free to [open a new issue](https://github.com/ginkgo-project/ginkgo/issues/new/choose) or ask a question using the [github discussions](https://github.com/ginkgo-project/ginkgo/discussions). Supported systems and requirements: + For all platforms, CMake 3.13+ + C++14 compliant compiler + Linux and macOS + GCC: 5.5+ + clang: 3.9+ + Intel compiler: 2018+ + Apple Clang: 14.0 is tested. Earlier versions might also work. + NVHPC: 22.7+ + Cray Compiler: 14.0.1+ + CUDA module: CUDA 9.2+ or NVHPC 22.7+ + HIP module: ROCm 4.5+ + DPC++ module: Intel OneAPI 2021.3+ with oneMKL and oneDPL. Set the CXX compiler to `dpcpp`. + Windows + MinGW: GCC 5.5+ + Microsoft Visual Studio: VS 2019+ + CUDA module: CUDA 9.2+, Microsoft Visual Studio + OpenMP module: MinGW. ### Version Support Changes + ROCm 4.0+ -> 4.5+ after [#1303](#1303) + Removed Cygwin pipeline and support [#1283](#1283) ### Interface Changes + Due to internal changes, `ConcreteExecutor::run` will now always throw if the corresponding module for the `ConcreteExecutor` is not build [#1234](#1234) + The constructor of `experimental::distributed::Vector` was changed to only accept local vectors as `std::unique_ptr` [#1284](#1284) + The default parameters for the `solver::MultiGrid` were improved. In particular, the smoother defaults to one iteration of `Ir` with `Jacobi` preconditioner, and the coarse grid solver uses the new direct solver with LU factorization. [#1291](#1291) [#1327](#1327) + The `iteration_complete` event gained a more expressive overload with additional parameters, the old overloads were deprecated. [#1288](#1288) [#1327](#1327) ### Deprecations + Deprecated less expressive `iteration_complete` event. Users are advised to now implement the function `void iteration_complete(const LinOp* solver, const LinOp* b, const LinOp* x, const size_type& it, const LinOp* r, const LinOp* tau, const LinOp* implicit_tau_sq, const array<stopping_status>* status, bool stopped)` [#1288](#1288) ### Added Features + A distributed Schwarz preconditioner. [#1248](#1248) + A GCR solver [#1239](#1239) + Flexible Gmres solver [#1244](#1244) + Enable Gmres solver for distributed matrices and vectors [#1201](#1201) + An example that uses Kokkos to assemble the system matrix [#1216](#1216) + A symbolic LU factorization allowing the `gko::experimental::factorization::Lu` and `gko::experimental::solver::Direct` classes to be used for matrices with non-symmetric sparsity pattern [#1210](#1210) + A numerical Cholesky factorization [#1215](#1215) + Symbolic factorizations in host-side operations are now wrapped in a host-side `Operation` to make their execution visible to loggers. This means that profiling loggers and benchmarks are no longer missing a separate entry for their runtime [#1232](#1232) + Symbolic factorization benchmark [#1302](#1302) + The `ProfilerHook` logger allows annotating the Ginkgo execution (apply, operations, ...) for profiling frameworks like NVTX, ROCTX and TAU. [#1055](#1055) + `ProfilerHook::created_(nested_)summary` allows the generation of a lightweight runtime profile over all Ginkgo functions written to a user-defined stream [#1270](#1270) for both host and device timing functionality [#1313](#1313) + It is now possible to enable host buffers for MPI communications at runtime even if the compile option `GINKGO_FORCE_GPU_AWARE_MPI` is set. [#1228](#1228) + A stencil matrices generator (5-pt, 7-pt, 9-pt, and 27-pt) for benchmarks [#1204](#1204) + Distributed benchmarks (multi-vector blas, SpMV, solver) [#1204](#1204) + Benchmarks for CSR sorting and lookup [#1219](#1219) + A timer for MPI benchmarks that reports the longest time [#1217](#1217) + A `timer_method=min|max|average|median` flag for benchmark timing summary [#1294](#1294) + Support for non-default streams in CUDA and HIP executors [#1236](#1236) + METIS integration for nested dissection reordering [#1296](#1296) + SuiteSparse AMD integration for fillin-reducing reordering [#1328](#1328) + Csr mixed-precision SpMV support [#1319](#1319) + A `with_loggers` function for all `Factory` parameters [#1337](#1337) ### Improvements + Improve naming of kernel operations for loggers [#1277](#1277) + Annotate solver iterations in `ProfilerHook` [#1290](#1290) + Allow using the profiler hooks and inline input strings in benchmarks [#1342](#1342) + Allow passing smart pointers in place of raw pointers to most matrix functions. This means that things like `vec->compute_norm2(x.get())` or `vec->compute_norm2(lend(x))` can be simplified to `vec->compute_norm2(x)` [#1279](#1279) [#1261](#1261) + Catch overflows in prefix sum operations, which makes Ginkgo's operations much less likely to crash. This also improves the performance of the prefix sum kernel [#1303](#1303) + Make the installed GinkgoConfig.cmake file relocatable and follow more best practices [#1325](#1325) ### Fixes + Fix OpenMPI version check [#1200](#1200) + Fix the mpi cxx type binding by c binding [#1306](#1306) + Fix runtime failures for one-sided MPI wrapper functions observed on some OpenMPI versions [#1249](#1249) + Disable thread pinning with GPU executors due to poor performance [#1230](#1230) + Fix hwloc version detection [#1266](#1266) + Fix PAPI detection in non-implicit include directories [#1268](#1268) + Fix PAPI support for newer PAPI versions: [#1321](#1321) + Fix pkg-config file generation for library paths outside prefix [#1271](#1271) + Fix various build failures with ROCm 5.4, CUDA 12, and OneAPI 6 [#1214](#1214), [#1235](#1235), [#1251](#1251) + Fix incorrect read for skew-symmetric MatrixMarket files with explicit diagonal entries [#1272](#1272) + Fix handling of missing diagonal entries in symbolic factorizations [#1263](#1263) + Fix segmentation fault in benchmark matrix construction [#1299](#1299) + Fix the stencil matrix creation for benchmarking [#1305](#1305) + Fix the additional residual check in IR [#1307](#1307) + Fix the cuSPARSE CSR SpMM issue on single strided vector when cuda >= 11.6 [#1322](#1322) [#1331](#1331) + Fix Isai generation for large sparsity powers [#1327](#1327) + Fix Ginkgo compilation and test with NVHPC >= 22.7 [#1331](#1331) + Fix Ginkgo compilation of 32 bit binaries with MSVC [#1349](#1349)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In #981, splitting the criterion for iteration and the residual check when users only pass the iteration criterion.
However, it will compute the residual norm from the solution in the iteration update when users pass the residual check.
It fixes the issue by only computing the residual from the solution in the finalized ste