solver (mostly krylov solver) and the residual norm with half#1711
Conversation
| } | ||
|
|
||
| GKO_INSTANTIATE_FOR_EACH_VALUE_TYPE(GKO_DECLARE_IDR_INITIALIZE_KERNEL); | ||
| GKO_INSTANTIATE_FOR_EACH_VALUE_TYPE_WITH_HALF( |
There was a problem hiding this comment.
A general question: Why do we need another macro _WITH_HALF ? Is it not possible to just have half within the first macro ?
There was a problem hiding this comment.
It is possible but it will result it in #1257 because we need to finish everything all at once.
There was a problem hiding this comment.
#1257 is to make everything work with half first and then disable some we can not support now.
The recent prs try to make the half support block by block. hopefully, they are easier for review.
To do it block by block, I need to create many list with half additionally unfortunately to reduce the touching other stuff.
MarcelKoch
left a comment
There was a problem hiding this comment.
LGTM. Can you clarify the testing issues with IDR?
f53438a to
5b22346
Compare
pratikvn
left a comment
There was a problem hiding this comment.
I think it makes sense to remove half support for solvers which do not converge with half ? (IDR, GCR ?) Otherwise, looks good to me.
| */ | ||
| template <typename ValueType> | ||
| inline std::enable_if_t<std::is_floating_point<ValueType>::value, ValueType> | ||
| inline std::enable_if_t<std::is_floating_point<ValueType>::value || |
There was a problem hiding this comment.
I think this structure will be useful everywhere. Maybe it would be nice to have our own is_floating_point which includes half ? So all you will need to do is gko::is_floating_point<ValueType>::value ?
There was a problem hiding this comment.
I only use it here because others usually need to be writen separately
| // if norm(t) is zero | ||
| // omega = 0 | ||
| // end if |
There was a problem hiding this comment.
I think this should be moved above and merged with the other definition of omega
|
|
||
| GKO_ASSERT_MTX_NEAR(x, l({1.5, 5.0, 2.0}), | ||
| (r_mixed<value_type, TypeParam>()) * 1e1); | ||
| (r_mixed<value_type, TypeParam>()) * 1e2); |
There was a problem hiding this comment.
It is enough to scale the old tolerance with alpha. We should actually do this for all solver advanced apply tests.
There was a problem hiding this comment.
I also check the other tol changes in this file. they are not needed at least in cuda10.2 image. need to check the other ci job
b4470ad to
ebb6468
Compare
a09d80a to
8f64d67
Compare
ff85d00 to
2aa59e2
Compare
3a98c11 to
ac216bc
Compare
Co-authored-by: Marcel Koch <marcel.koch@kit.edu>
|
Error: PR already merged! |
This PR enable the solvers and the residual norm with half. It also includes the corresponding config