Add VPs to SOECP #4375
Add VPs to SOECP #4375
Conversation
| R[ivp] = ref_pos + deltaV[ivp]; | ||
| { | ||
| R[ivp] = ref_pos + deltaV[ivp]; | ||
| spins[ivp] = ref_spin; //no spin deltas in this API |
There was a problem hiding this comment.
I'm not getting why spins needs to be updated. When not using spinor, Should the whole run not depending on the value of spins? Will there be any issue if we keep makeMoves untouched?
There was a problem hiding this comment.
Not for normal calculations. But for calculations that do include spinors yes it is needed. We still have NonLocalECPs used in spinor calculations, so if those evaluateRatios don't use VPs that have up to date spin values, the ratio evaluations are wrong
There was a problem hiding this comment.
Basically, if we don't have spinors then the original behavior of makeMoves would be fine. But if we do have spinors, then I need it to also have up to date spins (but no actual displacement from their original values).
I wasn't sure of the best way to handle this. Thats why I originally just passed the particleset by reference so I could hide the update inside VP instead of having to pass both the pos and spin as arguments
There was a problem hiding this comment.
For reference, if I don't keep the spins up to date in NonLocalECPComponent, then I get the following
mymac:build_gcc cmelton$ ctest -R SOREP-vmc
Test project /Users/cmelton/Software/qmcpack/build_gcc
Start 1286: short-bccMo_2x1x1_SOREP-vmc_sdj-1-16
1/24 Test #1286: short-bccMo_2x1x1_SOREP-vmc_sdj-1-16 ...................... Passed 81.97 sec
Start 1287: short-bccMo_2x1x1_SOREP-vmc_sdj-1-16-kinetic
2/24 Test #1287: short-bccMo_2x1x1_SOREP-vmc_sdj-1-16-kinetic .............. Passed 0.03 sec
Start 1288: short-bccMo_2x1x1_SOREP-vmc_sdj-1-16-totenergy
3/24 Test #1288: short-bccMo_2x1x1_SOREP-vmc_sdj-1-16-totenergy ............***Failed 0.02 sec
Start 1289: short-bccMo_2x1x1_SOREP-vmc_sdj-1-16-eeenergy
4/24 Test #1289: short-bccMo_2x1x1_SOREP-vmc_sdj-1-16-eeenergy ............. Passed 0.02 sec
Start 1290: short-bccMo_2x1x1_SOREP-vmc_sdj-1-16-samples
5/24 Test #1290: short-bccMo_2x1x1_SOREP-vmc_sdj-1-16-samples .............. Passed 0.02 sec
Start 1291: short-bccMo_2x1x1_SOREP-vmc_sdj-1-16-potential
6/24 Test #1291: short-bccMo_2x1x1_SOREP-vmc_sdj-1-16-potential ............***Failed 0.02 sec
Start 1292: short-bccMo_2x1x1_SOREP-vmc_sdj-1-16-ionion
7/24 Test #1292: short-bccMo_2x1x1_SOREP-vmc_sdj-1-16-ionion ............... Passed 0.02 sec
Start 1293: short-bccMo_2x1x1_SOREP-vmc_sdj-1-16-localecp
8/24 Test #1293: short-bccMo_2x1x1_SOREP-vmc_sdj-1-16-localecp ............. Passed 0.02 sec
Start 1294: short-bccMo_2x1x1_SOREP-vmc_sdj-1-16-nonlocalecp
9/24 Test #1294: short-bccMo_2x1x1_SOREP-vmc_sdj-1-16-nonlocalecp ..........***Failed 0.02 sec
Start 1295: short-bccMo_2x1x1_SOREP-vmc_sdj-1-16-soecp
10/24 Test #1295: short-bccMo_2x1x1_SOREP-vmc_sdj-1-16-soecp ................ Passed 0.02 sec
There was a problem hiding this comment.
Thanks for the info and I have a better picture now. I will make a follow up PR to consolidate APIs.
|
Test this please |
Proposed changes
This PR adds VirtualParticleSets to SOECPs. This essentially makes it so that we no longer have to add an additional keyword to run with SOECPs since it can now use the default batched path with virtual particles.
Note that while this is only implemented in the legacy APIs, this actually enables SOECPs to be used in the batched code since the default behavior of OperatorBase is to fallback to a loop over single walker APIs. So effectively, we have a path to run using the batched code, although it won't be as efficient as the mw implementation (coming soon).
I did a little refactoring in SOECP, and had to change the makeMoves API in VirtualParticleSet so that we also store the current electron spin...otherwise the NonLocalECP part was failing when spinors were enabled.
I also updated the short/long/determinanistic tests for bccMo to remove the algorithm=non-batched so that it uses the batched by default.
Also, I'm trying to make it so that I don't use raw pointers for the VirtualParticleSet (that's how it is still done in NonLocalECPComponent as well) and use smart pointers, but I ran into some hiccups as I was trying to change that and other things to use smart pointers. I'm still playing with that, but I wanted to go ahead and get this up so the rest of the code could be looked at.
What type(s) of changes does this code introduce?
Delete the items that do not apply
Does this introduce a breaking change?
What systems has this change been tested on?
my mac
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.