Use stedc instead of sterf in syevd #901
Conversation
tfalders
added
the
noOptimizations
tfalders
requested review from
jzuniga-amd,
cgmb,
qjojo,
EdDAzevedo,
jmachado-amd and
AGonzales-amd
as code owners
| EXPECT_ROCBLAS_STATUS(rocsolver_get_alg_mode(handle, rocsolver_function_sterf, &alg_mode), | ||
| rocblas_status_success); | ||
|
|
||
| EXPECT_EQ(alg_mode, rocsolver_alg_mode_hybrid); |
There was a problem hiding this comment.
Just double checking whether if alg_mode is set, it must be hybrid mode.
There was a problem hiding this comment.
Currently, alg_mode only has two valid values - hybrid (1) or not hybrid (0) - but you make a good point. To future proof it, I should probably check that it is actually equal to 1.
| @@ -103,11 +105,19 @@ class SYEVD_HEEVD : public ::TestWithParam<syevd_heevd_tuple> | |||
| } | |||
| }; | |||
|
|
|||
| class SYEVD : public SYEVD_HEEVD | |||
| class SYEVD : public SYEVD_HEEVD<0> | |||
There was a problem hiding this comment.
Just double checking whether "MODE" is an enum value such as rocsolver_alg_mode_hybrid. If so, perhaps it might be clearer to use the enum name. Just a thought.
There was a problem hiding this comment.
Hmmmm, it might be worth revisiting. IIRC, I made it 0 and 1 instead of an enum because it comes into rocsolver-bench that way, and also because other arguments in rocsolver_arguments.hpp are int types. Which aren't particularly compelling reasons to keep it that way.
| @@ -184,7 +177,7 @@ rocblas_status rocsolver_syevd_heevd_template(rocblas_handle handle, | |||
| strideE, tau, n, batch_count, scalars, (T*)work1, | |||
| (T*)work2, tmptau_W, workArr); | |||
|
|
|||
| if(evect != rocblas_evect_original) | |||
| if(sterf_mode == rocsolver_alg_mode_hybrid && evect != rocblas_evect_original) | |||
There was a problem hiding this comment.
Minor comment: do we need more enum option values for the new algorithm? Just a thought.
Performance numbers reveal that running syevd with stedc is orders of magnitude faster than with sterf, so this PR changes syevd to use stedc instead of sterf.
To preserve access to the numerical properties of sterf, syevd will still use sterf if hybrid sterf is enabled, since the performance difference there is much less.