Iterative refinement (IR) #243
Merged
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
gflegar
added
is:new-feature
gflegar
added
mod:cuda
tcojean
reviewed
Feb 27, 2019
|
|
||
| solver_->apply(lend(one_op), lend(residual), lend(one_op), dense_x); | ||
| residual->copy_from(dense_b); | ||
| system_matrix_->apply(lend(neg_one_op), dense_x, lend(one_op), |
There was a problem hiding this comment.
It's quite simple, but nonetheless I believe we in general put as comments the general algorithm of the solver, maybe you could also do that here? (or was that only for the step kernels?)
There was a problem hiding this comment.
it was only for the step kernels
hartwiganzt
reviewed
Feb 28, 2019
hartwiganzt
reviewed
Feb 28, 2019
hartwiganzt
reviewed
Feb 28, 2019
hartwiganzt
reviewed
Feb 28, 2019
hartwiganzt
reviewed
Feb 28, 2019
hartwiganzt
reviewed
Feb 28, 2019
hartwiganzt
reviewed
Feb 28, 2019
thoasm
approved these changes
Mar 4, 2019
| size_type num_cols, stopping_status *stop_status) | ||
| { | ||
| const auto tidx = | ||
| static_cast<size_type>(blockDim.x) * blockIdx.x + threadIdx.x; |
There was a problem hiding this comment.
This is the first time I see it handled that way (using explicit casting to size_type), what is the normal auto deduction? Is it not uint64?
tcojean
approved these changes
Mar 5, 2019
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.
I did a quick implementation of the iterative refinement method after getting back from work today (yesterday for you). It seemed like to much of a low hanging fruit not to have it in the first release.
Needles, to say, this does not try to do mixed precision IR (MPIR), but just the plain old IR, where you can specify what you want to use as the inner solver. By default I set it to the simplest possible thing - the identity operator, which results in Richardson iteration with scaling parameter 1.
To add mixed precision, we would have to solve the problem of conversions between value types - for that we would need to implement #247 - but even without it it's still useful, as it gives us Richardson iteration support, and if the default solver is replaced by the block-Jacobi precondtiioner, we actually get the block-Jacobi relaxation method (or what would be the "adaptive block-Jacobi relaxation method").
While writing the unit tests, I also figured out that
stopping_statusdid not have comparisons, so I added them, and while adding them I noticed that we had 2 methods for resetting the status (clear()andreset()), so I removedclear()and usedreset()whereverclear()was used previously.