Matrix update engines direct inversion

[PDoakORNL]

Proposed changes

Connect the CUDA direct inversion with minimum change to both develop and my design for performant and flexible acceleration of DiracDeterminantBatched.

What type(s) of changes does this code introduce?

• New feature
• Refactoring (no functional changes, no api changes)

Does this introduce a breaking change?

• No

What systems has this change been tested on?

Leconte

Checklist

Update the following with a yes where the items apply. If you're unsure about any of them, don't hesitate to ask. This is
simply a reminder of what we are going to look for before merging your code.

• Yes/No. This PR is up to date with current the current state of 'develop'
• Yes/No. Code added or changed in the PR has been clang-formatted
• Yes/No. This PR adds tests to cover any new code, or to catch a bug that is being fixed
• Yes/No. Documentation has been added (if appropriate)

[PDoakORNL]

Failing test is failing CI

[ye-luo]

Test this please

[ye-luo]

Investigating a test failure

[ye-luo]

MP passes test_wavefunction_trial but FP doesn't.
I think the failing was caused by DiracMatrixComputeCUDA.hpp

mw_invertTranspose(
      CUDALinearAlgebraHandles& cuda_handles,
      RefVector<DualMatrix<TMAT>>& a_mats,
      RefVector<DualMatrix<TMAT>>& inv_a_mats,
      DualVector<LogValue>& log_values,
      const std::vector<bool>& compute_mask)

1. a_mats are not padded. inv_a_mats are padded. So lda was incorrect in a few places. The expectation is using transpose to add padding before inversion.
2. This routine is expected to be synchronize the device and capture all error.
3. log_values is only needed on the host. So The dual space piece can be just internal to DiracMatrixComputeCUDA. Once this function is done, the device memory need to be up-to-date. Transfer batch will be handled by DDB. DDB should do D2H in evaluateLog but not in recompute used after branching.
4. Do we really need compute_mask?

Related to point 2 . cuBLAS_LU::computeInverseAndDetLog_batched calls cuBLAS::getri_batched directly but I found computeGetri_batched, I think we need to complete it by making it blocking and check infos.
I'm also thinking of fusing computeInverseAndDetLog_batched into DiracMatrixComputeCUDA.

[prckent]

not a change request: a_mats are not padded while inv_a_mats are. What is our logic for this (memory usage?) or is it a historical thing?

[ye-luo]

not a change request: a_mats are not padded while inv_a_mats are. What is our logic for this (memory usage?) or is it a historical thing?

It is more of a historic thing. When orbitals are produced by SPOSet, they are not padded thus a_mats are not padded.
inv_a_mats is padded because we frequently do vector operation on their rows.

[PDoakORNL]

Any batch you think is useless if for unit testing. They aren't useless unless you trust NVIDIA and AMD completely.

[PDoakORNL]

I will just drop the compute_mask from the API. My original direction on this was that all determinant data should be in per crowd data structures and not stored in the determinant, engine, or matrix objects. Didn't make much difference for async CUDA transfers so its premature optimization. I just didn't want to change it to get this code in and done diverging. Since this has already been delayed for a least a couple of months I might as well cut it.

[PDoakORNL]

So what is wrong with the unit test for test_DiracMatrixComputeCUDA, I don't think there is anything wrong with mw_invertTranspose at full precision. Looks to me like full precision mw_invertTranspose works perfectly as does the batched calculation of the log dets.

I will go over those unit tests again. I don't think its likely that is broken. So either something is up with the input or the higher level code expects additional state in the individual walkers to be updated in someway.

[ye-luo]

Any batch you think is useless if for unit testing. They aren't useless unless you trust NVIDIA and AMD completely.

I don't get what you mean here. We should have batching in unit tests. I don't even trust NVIDIA.

[PDoakORNL]

I see what you mean. I was mixing two issues here. And really those unit tests should be moved to the Platform unit tests. The computeGetrf_batched is only in the API for the unit test, otherwise it could just be in the .cu. The computeGetri_batched call only exists at all in either file for the unit test, nothing is done in the function, the unit test should also just call the cuBLAS wrapper itself. I'll make an issue for moving the cublas wrapper unit tests.

stale now

[ye-luo]

FYI. I ran one NiO a64 benchmark in mixed precision, I saw inversion was taking 7% of total time. Before this PR, the inversion time was 3x. Encouraging. I will do more tests once the non-deterministic fix in upstream is merged.

[PDoakORNL]

Ok by propagating the changes from #3514 I believe everything slated to possibly be const in the API is now const. So this is ready for be reviewed again.

[ye-luo]

Test this please

[PDoakORNL]

What the status on this? What is the status of the nondeterminism in develop?

[ye-luo]

I need to do a performance check. The additional transfer after matrix inversion concerns me but as long as it doesn't slowdown the full run too muc (<20%)h, we will merge and improve later. I will let you know in this week.

[ye-luo]

Start testing in-house

[ye-luo]

Pros. Acceleration is real
Cons. Need additional device memory.
I have prepared a PR for allowing selecting inverter from input. So users have larger degree of freedom.