Simplification of orbital rotations
- Dominant language
- C++
- Stars
- 403
- Forks
- 154
- Avg merge
- 1d 12h
- Merged PRs (30d)
- 82
Description
**Is your feature request related to a problem? Please describe.**
While we already got rid of the history method, I think there is more simplification in RotatedSPOSet that could be done. the kappa matrix, which defines the rotation parameters, form an anti-symmetric matrix, and the unitary rotation matrix which gets applied to the orbitals is formed from exponentiating this matrix. For a single determinant, the only rotations that impact the energy are core/virtual excitations, as core-core and virtual-virtual leave the energy unchanged.
When we construct the variational parameters in optimization, we only consider the core/virtual excitations since they are the only non-redundant parameters. However, during an optimization we continue to propose incremental changes to the rotation matrix which was applied in the previous iteration, something like exp(delta) exp(kappa) | psi >. Doing this over and over again, we end up with a final unitary matrix which is the product of all the small deltas throughout the optimization, i.e. exp(delta_N) exp(delta_N-1) ... exp(delta_0) |psi>. The history method kept track of this by storing all the deltas, while our global method stores the kappas to generate the current rotation matrix at every step of the optimization, so we do something like log(exp(delta) exp(kappa)) to get kappa'. It turns out that the kappa' from this operation now is a dense matrix, which looks like it has core-core and virtual-virtual rotations, even though we only update the core-virtual parameters. This is due to the operations not commuting.
However, if the core-core and virtual-virtual parameters really are redundant, it seems like we shouldn't need to deal with that complexity, and should just define the kappas as the sum of all the deltas, and the final unitary matrix just be exp(sum_i delta_i). Not only does it significantly simplify the source code, it also avoid potential numerical errors from constantly taking logs and exponentiating matrices over and over again.
Note that the original Berkeley implementation did exactly what i'm suggesting, and avoided all of this. Note that I have a branch where I've removed all of this and do the simple thing. I've tested OO using this approach, and the optimization completely agrees with an optimization keeping the full dense kappa, with slightly smaller fluctuations in the variance and energy. Also, I've taken a fully dense kappa and zeroed out the terms that come from core-core and virtual-virtual, and that agrees with VMC/DMC of the full kappa rotation. So it seems like doing the extra work only complicates the code, and doesn't actually give us any additional energy.
So I'm wanting to get opinions about simplifying the code. I do have a branch that implements this, and if people agree with the changes I could go ahead and PR it in. It seems like this was a premature optimization that wasn't actually necessary in the long run.
**Describe the solution you'd like**
A clear and concise description of what you want to happen.
**Describe alternatives you've considered**
A clear and concise description of any alternative solutions or features you've considered.
**Additional context**
Add any other context or screenshots about the feature request here.
Contributor guide
Assessment
This issue has not been assessed yet.