Fix Memory Model and Internal TRSM failure #852
Conversation
tfalders
added
the
noOptimizations
AGonzales-amd
requested review from
jzuniga-amd,
tfalders,
cgmb,
qjojo,
EdDAzevedo and
jmachado-amd
as code owners
| #ifndef USE_INTERNAL_TRSM | ||
| EXPECT_GT(size, 2000000); | ||
| #else | ||
| // internal trsm does not use scratch memory | ||
| EXPECT_LT(size, 2000000); | ||
| #endif |
There was a problem hiding this comment.
Hi @AGonzales-amd, those changes fix the failure, but I am afraid that they are not improving the test. The real problem is that this test is making assumptions about the implementation of getrf, and this is brittle: Unless it is absolutely necessary, we should not allow tests to make any assumptions about internal implementations. Implementations can change and render those assumptions redundant (which is exactly what is happening when getrf uses our TRSM).
A more appropriate solution would be to completely remove any calls to getrf that expect it to allocate an implementation dependent amount of memory, and make sure that the test itself sets everything it needs, explicitly.
| #else | ||
| // internal trsm does not use scratch memory | ||
| EXPECT_EQ(status, rocblas_status_success); | ||
| #endif |
There was a problem hiding this comment.
Minor questions: Is there a rocsolver function so the user can know say 100MB will be sufficient for the dgetrf rocsolver call for a certain matrix size, or this is just trial-and-error or keep doubling the size until it works? Perhaps when using rocblas managed mode, there is a way to query the max memory pool size used? Perhaps the user can use rocblas managed mode, then later switch to user owned mode (but knowing the max pool size used in rocblas) for higher performance?
This PR fixes the
checkin_misc_MEMORY_MODEL.user_managedtest failure whenROCSOLVER_USE_INTERNAL_BLASis set and internal trsm is used forrocsolver_dgetrf_strided_batched.