Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add VPs to SOECP #4375

Merged
merged 12 commits into from
Dec 21, 2022
31 changes: 30 additions & 1 deletion src/Particle/VirtualParticleSet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ void VirtualParticleSet::releaseResource(ResourceCollection& collection,
/// move virtual particles to new postions and update distance tables
void VirtualParticleSet::makeMoves(int jel,
const PosType& ref_pos,
const RealType ref_spin,
const std::vector<PosType>& deltaV,
bool sphere,
int iat)
Expand All @@ -112,7 +113,35 @@ void VirtualParticleSet::makeMoves(int jel,
refSourcePtcl = iat;
assert(R.size() == deltaV.size());
for (size_t ivp = 0; ivp < R.size(); ivp++)
R[ivp] = ref_pos + deltaV[ivp];
{
R[ivp] = ref_pos + deltaV[ivp];
spins[ivp] = ref_spin; //no spin deltas in this API
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

@camelto2 camelto2 Dec 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@camelto2 camelto2 Dec 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the info and I have a better picture now. I will make a follow up PR to consolidate APIs.

}
update();
}

/// move virtual particles to new postions and update distance tables
void VirtualParticleSet::makeMovesWithSpin(int jel,
const PosType& ref_pos,
const RealType ref_spin,
const std::vector<PosType>& deltaV,
const std::vector<RealType>& deltaS,
bool sphere,
int iat)
{
if (sphere && iat < 0)
throw std::runtime_error(
"VirtualParticleSet::makeMovesWithSpin is invoked incorrectly, the flag sphere=true requires iat specified!");
onSphere = sphere;
refPtcl = jel;
refSourcePtcl = iat;
assert(R.size() == deltaV.size());
assert(spins.size() == deltaS.size());
for (size_t ivp = 0; ivp < R.size(); ivp++)
{
R[ivp] = ref_pos + deltaV[ivp];
spins[ivp] = ref_spin + deltaS[ivp];
}
update();
}

Expand Down
19 changes: 19 additions & 0 deletions src/Particle/VirtualParticleSet.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,16 +76,35 @@ class VirtualParticleSet : public ParticleSet
/** move virtual particles to new postions and update distance tables
* @param jel reference particle that all the VP moves from
* @param ref_pos reference particle position
* @param ref_spin reference particle spin
* @param deltaV Position delta for virtual moves.
* @param sphere set true if VP are on a sphere around the reference source particle
* @param iat reference source particle
*/
void makeMoves(int jel,
const PosType& ref_pos,
const RealType ref_spin,
const std::vector<PosType>& deltaV,
bool sphere = false,
int iat = -1);

/** move virtual particles to new postions and update distance tables
* @param jel reference particle that all the VP moves from
* @param ref_pos reference particle position
* @param ref_spin reference particle spin
* @param deltaV Position delta for virtual moves.
* @param deltaS Spin delta for virtual moves.
* @param sphere set true if VP are on a sphere around the reference source particle
* @param iat reference source particle
*/
void makeMovesWithSpin(int jel,
const PosType& ref_pos,
const RealType ref_spin,
const std::vector<PosType>& deltaV,
const std::vector<RealType>& deltaS,
bool sphere = false,
int iat = -1);

static void mw_makeMoves(const RefVectorWithLeader<VirtualParticleSet>& vp_list,
const RefVector<const std::vector<PosType>>& deltaV_list,
const RefVector<const NLPPJob<RealType>>& joblist,
Expand Down
13 changes: 6 additions & 7 deletions src/QMCHamiltonians/ECPotentialBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -144,12 +144,7 @@ bool ECPotentialBuilder::put(xmlNodePtr cur)
{
nknot_max = std::max(nknot_max, nonLocalPot[i]->getNknot());
if (NLPP_algo == "batched")
{
if( !targetPtcl.isSpinor())
nonLocalPot[i]->initVirtualParticle(targetPtcl);
else
throw std::runtime_error("Batched NLPP evaluation not validated with spinors. Use algorithm=\"non-batched\" in pseudopotential block.");
}
nonLocalPot[i]->initVirtualParticle(targetPtcl);
apot->addComponent(i, std::move(nonLocalPot[i]));
}
}
Expand Down Expand Up @@ -181,12 +176,16 @@ bool ECPotentialBuilder::put(xmlNodePtr cur)
{
nknot_max = std::max(nknot_max, soPot[i]->getNknot());
sknot_max = std::max(sknot_max, soPot[i]->getSknot());
if (NLPP_algo == "batched")
soPot[i]->initVirtualParticle(targetPtcl);
apot->addComponent(i, std::move(soPot[i]));
}
}
app_log() << "\n Using SOECP potential \n"
<< " Maximum grid on a sphere for SOECPotential: " << nknot_max << std::endl;
app_log() << " Maximum grid for Simpson's rule for spin integral: " << sknot_max << std::endl;
if (NLPP_algo == "batched")
app_log() << " Using batched ratio computing in SOECP potential" << std::endl;

if (physicalSO == "yes")
targetH.addOperator(std::move(apot), "SOECP"); //default is physical operator
Expand Down Expand Up @@ -218,7 +217,7 @@ void ECPotentialBuilder::useXmlFormat(xmlNodePtr cur)
std::string href("none");
std::string ionName("none");
std::string format("xml");
int nrule = -1;
int nrule = -1;
int llocal = -1;
//RealType rc(2.0);//use 2 Bohr
OhmmsAttributeSet hAttrib;
Expand Down
7 changes: 3 additions & 4 deletions src/QMCHamiltonians/NonLocalECPComponent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ NonLocalECPComponent::RealType NonLocalECPComponent::evaluateOne(ParticleSet& W,
if (VP)
{
// Compute ratios with VP
VP->makeMoves(iel, W.R[iel], deltaV, true, iat);
VP->makeMoves(iel, W.R[iel], W.spins[iel], deltaV, true, iat);
if (use_DLA)
psi.evaluateRatios(*VP, psiratio, TrialWaveFunction::ComputeType::FERMIONIC);
else
Expand Down Expand Up @@ -310,7 +310,7 @@ NonLocalECPComponent::RealType NonLocalECPComponent::evaluateOneWithForces(Parti
{
APP_ABORT("NonLocalECPComponent::evaluateOneWithForces(...): Forces not implemented with virtual particle moves\n");
// Compute ratios with VP
VP->makeMoves(iel, W.R[iel], deltaV, true, iat);
VP->makeMoves(iel, W.R[iel], W.spins[iel], deltaV, true, iat);
psi.evaluateRatios(*VP, psiratio);
}
else
Expand Down Expand Up @@ -460,7 +460,7 @@ NonLocalECPComponent::RealType NonLocalECPComponent::evaluateOneWithForces(Parti
{
APP_ABORT("NonLocalECPComponent::evaluateOneWithForces(...): Forces not implemented with virtual particle moves\n");
// Compute ratios with VP
VP->makeMoves(iel, W.R[iel], deltaV, true, iat);
VP->makeMoves(iel, W.R[iel], W.spins[iel], deltaV, true, iat);
psi.evaluateRatios(*VP, psiratio);
}
else
Expand Down Expand Up @@ -802,7 +802,6 @@ void NonLocalECPComponent::evaluateOneBodyOpMatrixdRContribution(ParticleSet& W,

for (int j = 0; j < nknot; j++)
{

RealType zz = dot(dr, rrotsgrid_m[j]) * rinv;
PosType uminusrvec = rrotsgrid_m[j] - zz * dr * rinv;

Expand Down
2 changes: 1 addition & 1 deletion src/QMCHamiltonians/NonLocalECPotential.deriv.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ NonLocalECPComponent::RealType NonLocalECPComponent::evaluateValueAndDerivatives
if (VP)
{
// Compute ratios with VP
VP->makeMoves(iel, W.R[iel], deltaV, true, iat);
VP->makeMoves(iel, W.R[iel], W.spins[iel], deltaV, true, iat);
psi.evaluateDerivRatios(*VP, optvars, psiratio, dratio);
}
else
Expand Down
77 changes: 63 additions & 14 deletions src/QMCHamiltonians/SOECPComponent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,33 @@

namespace qmcplusplus
{
SOECPComponent::SOECPComponent() : lmax(0), nchannel(0), nknot(0), sknot(0), Rmax(-1) {}
SOECPComponent::SOECPComponent() : lmax(0), nchannel(0), nknot(0), sknot(0), Rmax(-1), VP(nullptr) {}

SOECPComponent::~SOECPComponent()
{
for (int i = 0; i < sopp_m.size(); i++)
delete sopp_m[i];
if (VP)
delete VP;
}

void SOECPComponent::print(std::ostream& os) {}

void SOECPComponent::initVirtualParticle(const ParticleSet& qp)
{
assert(VP == nullptr);
outputManager.pause();
VP = new VirtualParticleSet(qp, nknot);
outputManager.resume();
}

void SOECPComponent::deleteVirtualParticle()
{
if (VP)
delete VP;
VP = nullptr;
}

void SOECPComponent::add(int l, RadialPotentialType* pp)
{
angpp_m.push_back(l);
Expand All @@ -38,13 +55,16 @@ SOECPComponent* SOECPComponent::makeClone(const ParticleSet& qp)
SOECPComponent* myclone = new SOECPComponent(*this);
for (int i = 0; i < sopp_m.size(); i++)
myclone->sopp_m[i] = sopp_m[i]->makeClone();
if (VP)
myclone->VP = new VirtualParticleSet(qp, nknot);
return myclone;
}

void SOECPComponent::resize_warrays(int n, int m, int s)
{
psiratio.resize(n);
deltaV.resize(n);
deltaS.resize(n);
vrad.resize(m);
rrotsgrid_m.resize(n);
nchannel = sopp_m.size();
Expand Down Expand Up @@ -108,18 +128,34 @@ SOECPComponent::ComplexType SOECPComponent::getAngularIntegral(RealType sold,
TrialWaveFunction& Psi,
int iel,
RealType r,
const PosType& dr)
const PosType& dr,
int iat)
{
//quadrature sum for angular integral
constexpr RealType fourpi = 2.0 * TWOPI;
for (int j = 0; j < nknot; j++)
buildQuadraturePointDeltas(r, dr, deltaV, snew - sold, deltaS);

if (VP)
{
deltaV[j] = r * rrotsgrid_m[j] - dr;
W.makeMoveWithSpin(iel, deltaV[j], snew - sold);
psiratio[j] = Psi.calcRatio(W, iel) * sgridweight_m[j] * fourpi;
W.rejectMove(iel);
Psi.resetPhaseDiff();
VP->makeMovesWithSpin(iel, W.R[iel], W.spins[iel], deltaV, deltaS, true, iat);
Psi.evaluateRatios(*VP, psiratio);
}
else
{
//quadrature sum for angular integral
for (int j = 0; j < nknot; j++)
{
W.makeMoveWithSpin(iel, deltaV[j], deltaS[j]);
psiratio[j] = Psi.calcRatio(W, iel);
W.rejectMove(iel);
Psi.resetPhaseDiff();
}
}

//Need to add appropriate weight to psiratio
std::transform(psiratio.begin(), psiratio.end(), sgridweight_m.begin(), psiratio.begin(),
[](auto& psi, auto& weight) {
RealType fourpi = 2.0 * TWOPI;
return psi * weight * fourpi;
});

ComplexType angint(0.0);
for (int j = 0; j < nknot; j++)
Expand Down Expand Up @@ -176,22 +212,35 @@ SOECPComponent::RealType SOECPComponent::evaluateOne(ParticleSet& W,
for (int is = 1; is <= sknot - 1; is += 2)
{
RealType snew = smin + is * dS;
ComplexType angint = getAngularIntegral(sold, snew, W, Psi, iel, r, dr);
ComplexType angint = getAngularIntegral(sold, snew, W, Psi, iel, r, dr, iat);
sint += RealType(4. / 3.) * dS * angint;
}
for (int is = 2; is <= sknot - 2; is += 2)
{
RealType snew = smin + is * dS;
ComplexType angint = getAngularIntegral(sold, snew, W, Psi, iel, r, dr);
ComplexType angint = getAngularIntegral(sold, snew, W, Psi, iel, r, dr, iat);
sint += RealType(2. / 3.) * dS * angint;
}
sint += RealType(1. / 3.) * dS * getAngularIntegral(sold, smin, W, Psi, iel, r, dr);
sint += RealType(1. / 3.) * dS * getAngularIntegral(sold, smax, W, Psi, iel, r, dr);
sint += RealType(1. / 3.) * dS * getAngularIntegral(sold, smin, W, Psi, iel, r, dr, iat);
sint += RealType(1. / 3.) * dS * getAngularIntegral(sold, smax, W, Psi, iel, r, dr, iat);

RealType pairpot = std::real(sint) / TWOPI;
return pairpot;
}

void SOECPComponent::buildQuadraturePointDeltas(RealType r,
const PosType& dr,
std::vector<PosType>& deltaV,
RealType ds,
std::vector<RealType>& deltaS) const
{
for (int j = 0; j < nknot; j++)
{
deltaV[j] = r * rrotsgrid_m[j] - dr;
deltaS[j] = ds;
}
}

void SOECPComponent::randomize_grid(RandomGenerator& myRNG)
{
RealType phi(TWOPI * myRNG()), psi(TWOPI * myRNG()), cth(myRNG() - 0.5);
Expand Down
14 changes: 12 additions & 2 deletions src/QMCHamiltonians/SOECPComponent.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,15 +60,24 @@ class SOECPComponent : public QMCTraits
TrialWaveFunction& Psi,
int iel,
RealType r,
const PosType& dr);
const PosType& dr,
int iat);

std::vector<PosType> deltaV;
std::vector<RealType> deltaS;
SpherGridType sgridxyz_m;
SpherGridType rrotsgrid_m;
std::vector<ValueType> psiratio;
std::vector<ValueType> vrad;
std::vector<RealType> sgridweight_m;

VirtualParticleSet* VP;

void buildQuadraturePointDeltas(RealType r,
const PosType& dr,
std::vector<PosType>& deltaV,
RealType ds,
std::vector<RealType>& deltaS) const;

public:
SOECPComponent();
Expand Down Expand Up @@ -116,7 +125,8 @@ class SOECPComponent : public QMCTraits

void print(std::ostream& os);

//void initVirtualParticle(const ParticleSet& qp){};
void initVirtualParticle(const ParticleSet& qp);
void deleteVirtualParticle();

inline void setRmax(int rmax) { Rmax = rmax; }
inline RealType getRmax() const { return Rmax; }
Expand Down
29 changes: 18 additions & 11 deletions src/QMCHamiltonians/tests/test_ecp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -536,21 +536,28 @@ TEST_CASE("Evaluate_soecp", "[hamiltonian]")
//Need to set up temporary data for this configuration in trial wavefunction. Needed for ratios.
double logpsi = psi.evaluateLog(elec);

RealType Value1(0.0);

for (int jel = 0; jel < elec.getTotalNum(); jel++)
{
const auto& dist = myTable.getDistRow(jel);
const auto& displ = myTable.getDisplRow(jel);
for (int iat = 0; iat < ions.getTotalNum(); iat++)
auto test_evaluateOne = [&]() {
RealType Value1(0.0);
for (int jel = 0; jel < elec.getTotalNum(); jel++)
{
if (sopp != nullptr && dist[iat] < sopp->getRmax())
{
Value1 += sopp->evaluateOne(elec, iat, psi, jel, dist[iat], RealType(-1) * displ[iat]);
}
const auto& dist = myTable.getDistRow(jel);
const auto& displ = myTable.getDisplRow(jel);
for (int iat = 0; iat < ions.getTotalNum(); iat++)
if (sopp != nullptr && dist[iat] < sopp->getRmax())
Value1 += sopp->evaluateOne(elec, iat, psi, jel, dist[iat], RealType(-1) * displ[iat]);
}
REQUIRE(Value1 == Approx(-0.3214176962));
};

{
//test with VPs
sopp->initVirtualParticle(elec);
test_evaluateOne();
sopp->deleteVirtualParticle();
//test without VPs
test_evaluateOne();
}
REQUIRE(Value1 == Approx(-0.3214176962));
}
#endif

Expand Down
2 changes: 1 addition & 1 deletion src/QMCWaveFunctions/tests/test_DiracDeterminant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ void test_DiracDeterminant_first(const DetMatInvertor inverter_kind)
std::vector<ValueType> ratios2(2);
newpos2[0] = newpos - elec.R[1];
newpos2[1] = PosType(0.2, 0.5, 0.3) - elec.R[1];
VP.makeMoves(1, elec.R[1], newpos2);
VP.makeMoves(1, elec.R[1], elec.spins[1], newpos2);
ddb.evaluateRatios(VP, ratios2);

CHECK(std::real(ratios2[0]) == Approx(0.4880285278));
Expand Down
Loading