Half factorization #1712
Half factorization #1712
Conversation
ginkgo-bot
added
reg:testing
yhmtsai
force-pushed
the
half_factorization
branch
from
3db59fd
to
cd9677a
Compare
yhmtsai
force-pushed
the
half_factorization
branch
from
cd9677a
to
5e5cd03
Compare
yhmtsai
force-pushed
the
half_factorization
branch
from
5e5cd03
to
c276034
Compare
yhmtsai
force-pushed
the
half_factorization
branch
from
c276034
to
bbefde6
Compare
yhmtsai
force-pushed
the
half_factorization
branch
from
bbefde6
to
72d9d50
Compare
yhmtsai
force-pushed
the
half_factorization
branch
from
72d9d50
to
88967e6
Compare
yhmtsai
added
1:ST:ready-for-review
yhmtsai
force-pushed
the
half_factorization
branch
from
88967e6
to
e667ec0
Compare
yhmtsai
force-pushed
the
half_solver
branch
2 times, most recently
from
50ae4c1
to
bba40e0
Compare
yhmtsai
force-pushed
the
half_factorization
branch
from
e667ec0
to
c32201d
Compare
There was a problem hiding this comment.
Generally LGTM. I have a question regarding atomics and hip. The latest ROCm shows support for fp16 atomic operations: https://rocm.docs.amd.com/en/latest/reference/precision-support.html#atomic-operations-support, but TBH I can't figure out what operations exactly they mean with that. Did you try anything in that regard?
| PairTypenameNameGenerator); | ||
|
|
||
|
|
||
| TYPED_TEST(ParIlut, KernelThresholdSelectIsEquivalentToRef) | ||
| { | ||
| using value_type = typename TestFixture::value_type; |
There was a problem hiding this comment.
Many of the tests here are missing SKIP_HALF if compiling for HIP.
There was a problem hiding this comment.
we do not support compute_l_u_factors in hip, but the others still works with half precision in HIP
cuda/solver/common_trs_kernels.cuh
Outdated
| @@ -212,13 +212,15 @@ struct CudaSolveStruct : gko::solver::SolveStruct { | |||
|
|
|||
| size_type work_size{}; | |||
|
|
|||
| // TODO: In nullptr is considered nullptr_t not casted to const | |||
| // it does not work in cuda110/100 images | |||
There was a problem hiding this comment.
nit:
| // it does not work in cuda110/100 images | |
| // Explicitly cast `nullptr` to `const ValueType*` to prevent compiler issues with cuda 10/11 |
There was a problem hiding this comment.
I think it is more on the host compiler side because it goes through our binding first with specfic type
yhmtsai
force-pushed
the
half_factorization
branch
from
c32201d
to
257585d
Compare
cuda/solver/common_trs_kernels.cuh
Outdated
| using shared_value_type = std::conditional_t< | ||
| std::is_same<remove_complex<ValueType>, gko::half>::value, float, | ||
| ValueType>; | ||
| sptrsv_naive_caching_kernel<is_upper, device_type<shared_value_type>> |
There was a problem hiding this comment.
now this will also be float when using std::complex<gko::half>. That doesn't seem correct. You will loose any imaginary part that might be in the matrix or vectors.
There was a problem hiding this comment.
you are right.
Sorry, I do not pay enough attention when changing it.
yhmtsai
force-pushed
the
half_factorization
branch
from
257585d
to
eb14467
Compare
yhmtsai
force-pushed
the
half_factorization
branch
2 times, most recently
from
7568854
to
d68a589
Compare
yhmtsai
force-pushed
the
half_factorization
branch
from
d68a589
to
e1a3b3d
Compare
yhmtsai
force-pushed
the
half_factorization
branch
2 times, most recently
from
bea709e
to
e4973cb
Compare
… in shared memory
yhmtsai
force-pushed
the
half_factorization
branch
from
e4973cb
to
f6291e6
Compare
this pr adds the factorization with half support.
Hip does not support atomic on the 16bits type currently
TODO: