deepmodeling
Confusing Interface of hPsi
Describe the Code Quality Issue
Description
In module HSolver, we need hPsi as a function to perform a matrix-blockvector multiplication on input psi.
We need to perform Matrix-Vector multiplication from index1 to index2:
$$hpsi[id1:id2]=H * psi[id1:id2]$$
One intuitive way to do this is to pass in the input/output pointer(head address) and the corresponding indices:
hpsi(in, out, id1, id2) to do mv on in[id1:id2] and write the results to out[id1:id2].
However, it seems that current implemention requires the calling function to handle input pointer along with its offset, i.e. we need to call hpsi as follows:
hpsi(in, &out[id1], id1, id2) or hpsi(in, out+id1*ldOUT, id1, id2). This is confusing and error-prone.
Is it possible to change the interface of hpsi function to make it clearer?
Related issues
#4961 When working on modularization of dav&dav_subspace, we discovered this inconsistency, leading to some segfaults due to inproper function calls.
Additional Context
No response
Task list for Issue attackers (only for developers)
- [ ] Identify the specific code file or section with the code quality issue.
- [ ] Investigate the issue and determine the root cause.
- [ ] Research best practices and potential solutions for the identified issue.
- [ ] Refactor the code to improve code quality, following the suggested solution.
- [ ] Ensure the refactored code adheres to the project's coding standards.
- [ ] Test the refactored code to ensure it functions as expected.
- [ ] Update any relevant documentation, if necessary.
- [ ] Submit a pull request with the refactored code and a description of the changes made.
I'll try to create a PR to change hpsi_func interface in hsolver to solve this issue.
As suggested by @Qianruipku, sometimes one may want to store the result of HX in a temporary array. We will have a further discussion on this.
I will try to refactor hpsi_func to be called as hpsi(X, HX, leading dimension, n_vectors),
where X and HX are exactly the address to read and write from, so that it will be intuitive.
Calling functions in eigensolvers will do it like hpsi_func(psi[start_index], hpsi[start_index], leading dimension, n_vectors).