From 16845f98d527c6746ea2ba6924052f2a4acc5774 Mon Sep 17 00:00:00 2001 From: Ye Luo Date: Tue, 26 Feb 2019 00:31:16 -0600 Subject: [PATCH 1/4] Reduce unnecessary interfaces in DelayedUpdate. Move the state machine out to DiracDeterminant. --- src/QMCWaveFunctions/Fermion/DelayedUpdate.h | 74 ++----------------- .../Fermion/DiracDeterminant.cpp | 33 +++++++-- .../Fermion/DiracDeterminant.h | 10 +++ .../tests/test_dirac_matrix.cpp | 6 +- 4 files changed, 46 insertions(+), 77 deletions(-) diff --git a/src/QMCWaveFunctions/Fermion/DelayedUpdate.h b/src/QMCWaveFunctions/Fermion/DelayedUpdate.h index 3075070720..f72eaa3631 100644 --- a/src/QMCWaveFunctions/Fermion/DelayedUpdate.h +++ b/src/QMCWaveFunctions/Fermion/DelayedUpdate.h @@ -41,20 +41,9 @@ namespace qmcplusplus { /// current number of delays, increase one for each acceptance, reset to 0 after updating Ainv int delay_count; - /// pointer to the row of up-to-date Ainv - const T* Ainv_row_ptr; - /** row id correspond to the up-to-date Ainv_row_ptr. [0 norb), Ainv_row_ptr is valid; -1, Ainv_row_ptr is not valid. - * This id is set by getInvRow indicating Ainv_row_ptr has been prepared for the Ainv_row_id row - * ratioGrad checks if Ainv_row_id is consistent. If not, Ainv_row_ptr needs to be recomputed. - * acceptRow and updateInvMat mark Ainv_row_ptr invalid by setting Ainv_row_id to -1 - */ - int Ainv_row_id; - /// current determinant ratio - T curRatio; - public: /// default constructor - DelayedUpdate(): delay_count(0), Ainv_row_ptr(nullptr), Ainv_row_id(-1) {} + DelayedUpdate(): delay_count(0) {} ///resize the internal storage /** resize the internal storage @@ -76,13 +65,13 @@ namespace qmcplusplus { * @param Ainv inverse matrix * @param rowchanged the row id corresponding to the proposed electron */ - inline void getInvRow(const Matrix& Ainv, int rowchanged) + template + inline void getInvRow(const Matrix& Ainv, int rowchanged, VVT& invRow) { - Ainv_row_id = rowchanged; if ( delay_count == 0 ) { // Ainv is fresh, directly access Ainv - Ainv_row_ptr = Ainv[rowchanged]; + std::copy_n(Ainv[rowchanged], invRow.size(), invRow.data()); return; } const T cone(1); @@ -91,57 +80,11 @@ namespace qmcplusplus { const int norb = Ainv.rows(); const int lda_Binv = Binv.cols(); // save AinvRow to new_AinvRow - std::copy_n(AinvRow, norb, V[delay_count]); + std::copy_n(AinvRow, norb, invRow.data()); // multiply V (NxK) Binv(KxK) U(KxN) AinvRow right to the left BLAS::gemv('T', norb, delay_count, cone, U.data(), norb, AinvRow, 1, czero, p.data(), 1); BLAS::gemv('N', delay_count, delay_count, cone, Binv.data(), lda_Binv, p.data(), 1, czero, Binv[delay_count], 1); - BLAS::gemv('N', norb, delay_count, -cone, V.data(), norb, Binv[delay_count], 1, cone, V[delay_count], 1); - Ainv_row_ptr = V[delay_count]; - } - - /** compute determinant ratio of new determinant - * @param Ainv inverse matrix - * @param rowchanged the row id corresponding to the proposed electron - * @param psiV new orbital values - */ - template - inline T ratio(const Matrix& Ainv, int rowchanged, const VVT& psiV) - { - // check Ainv_row_id against rowchanged to see if getInvRow() has been called - // This is intended to save redundant compuation in TM1 and TM3 - if(Ainv_row_id != rowchanged) - getInvRow(Ainv, rowchanged); - return curRatio = simd::dot(Ainv_row_ptr,psiV.data(),Ainv.cols()); - } - - /** compute the old gradient - * @param Ainv inverse matrix - * @param rowchanged the row id corresponding to the proposed electron - * @param dpsiV old orbital derivatives - */ - template - inline GT evalGrad(const Matrix& Ainv, int rowchanged, const GT* dpsiV) - { - getInvRow(Ainv, rowchanged); - return simd::dot(Ainv_row_ptr,dpsiV,Ainv.cols()); - } - - /** compute determinant ratio and gradients of new determinant - * @param Ainv inverse matrix - * @param rowchanged the row id corresponding to the proposed electron - * @param psiV new orbital values - * @param dpsiV new orbital derivatives - * @param g new gradients - */ - template - inline T ratioGrad(const Matrix& Ainv, int rowchanged, const VVT& psiV, const GGT& dpsiV, GT& g) - { - // check Ainv_row_id against rowchanged to ensure getInvRow() called before ratioGrad() - // This is not a safety check. Some code paths do call ratioGrad without calling evalGrad first. - if(Ainv_row_id != rowchanged) - getInvRow(Ainv, rowchanged); - g = simd::dot(Ainv_row_ptr,dpsiV.data(),Ainv.cols()); - return curRatio = simd::dot(Ainv_row_ptr,psiV.data(),Ainv.cols()); + BLAS::gemv('N', norb, delay_count, -cone, V.data(), norb, Binv[delay_count], 1, cone, invRow.data(), 1); } /** accept a move with the update delayed @@ -154,9 +97,6 @@ namespace qmcplusplus { template inline void acceptRow(Matrix& Ainv, int rowchanged, const VVT& psiV) { - // Ainv_row_ptr is no more valid by marking Ainv_row_id -1 - Ainv_row_id = -1; - const T cminusone(-1); const T czero(0); const int norb = Ainv.rows(); @@ -188,8 +128,6 @@ namespace qmcplusplus { */ inline void updateInvMat(Matrix& Ainv) { - // Ainv_row_ptr is no more valid by marking Ainv_row_id -1 - Ainv_row_id = -1; if(delay_count==0) return; // update the inverse matrix const T cone(1); diff --git a/src/QMCWaveFunctions/Fermion/DiracDeterminant.cpp b/src/QMCWaveFunctions/Fermion/DiracDeterminant.cpp index c441d420d4..bf535d97a6 100644 --- a/src/QMCWaveFunctions/Fermion/DiracDeterminant.cpp +++ b/src/QMCWaveFunctions/Fermion/DiracDeterminant.cpp @@ -32,7 +32,7 @@ namespace qmcplusplus *@param first index of the first particle */ DiracDeterminant::DiracDeterminant(SPOSetPtr const spos, int first): - DiracDeterminantBase(spos,first), ndelay(1) + DiracDeterminantBase(spos,first), ndelay(1), invRow_id(-1) { ClassName = "DiracDeterminant"; } @@ -87,6 +87,7 @@ void DiracDeterminant::resize(int nel, int morb) dpsiM.resize(nel,norb); d2psiM.resize(nel,norb); psiV.resize(norb); + invRow.resize(norb); psiM_temp.resize(nel,norb); if( typeid(ValueType) != typeid(mValueType) ) psiM_hp.resize(nel,norb); @@ -116,7 +117,9 @@ DiracDeterminant::evalGrad(ParticleSet& P, int iat) { const int WorkingIndex = iat-FirstIndex; RatioTimer.start(); - GradType g = updateEng.evalGrad(psiM, WorkingIndex, dpsiM[WorkingIndex]); + updateEng.getInvRow(psiM, WorkingIndex, invRow); + invRow_id = WorkingIndex; + GradType g = simd::dot(invRow.data(), dpsiM[WorkingIndex], invRow.size()); RatioTimer.stop(); return g; } @@ -131,9 +134,16 @@ DiracDeterminant::ratioGrad(ParticleSet& P, int iat, GradType& grad_iat) const int WorkingIndex = iat-FirstIndex; UpdateMode=ORB_PBYP_PARTIAL; GradType rv; - curRatio = updateEng.ratioGrad(psiM, WorkingIndex, psiV, dpsiV, rv); - grad_iat += ((RealType)1.0/curRatio) * rv; - RatioTimer.stop(); + + // check invRow_id against WorkingIndex to ensure getInvRow() called before ratioGrad() + // This is not a safety check. Some code paths do call ratioGrad without calling evalGrad first. + if(invRow_id != WorkingIndex) + { + updateEng.getInvRow(psiM, WorkingIndex, invRow); + invRow_id = WorkingIndex; + } + curRatio = simd::dot(invRow.data(), psiV.data(), invRow.size()); + grad_iat += ((RealType)1.0/curRatio) * simd::dot(invRow.data(), dpsiV.data(), invRow.size()); return curRatio; } @@ -146,6 +156,8 @@ void DiracDeterminant::acceptMove(ParticleSet& P, int iat) LogValue +=std::log(std::abs(curRatio)); UpdateTimer.start(); updateEng.acceptRow(psiM,WorkingIndex,psiV); + // invRow becomes invalid after accepting a move + invRow_id = -1; if(UpdateMode == ORB_PBYP_PARTIAL) { simd::copy(dpsiM[WorkingIndex], dpsiV.data(), NumOrbitals); @@ -165,6 +177,8 @@ void DiracDeterminant::restore(int iat) void DiracDeterminant::completeUpdates() { UpdateTimer.start(); + // invRow becomes invalid after updating the inverse matrix + invRow_id = -1; updateEng.updateInvMat(psiM); UpdateTimer.stop(); } @@ -265,7 +279,14 @@ DiracDeterminant::ValueType DiracDeterminant::ratio(ParticleSet& P, int iat) Phi->evaluate(P, iat, psiV); SPOVTimer.stop(); RatioTimer.start(); - curRatio = updateEng.ratio(psiM, WorkingIndex, psiV); + // check invRow_id against WorkingIndex to see if getInvRow() has been called + // This is intended to save redundant compuation in TM1 and TM3 + if(invRow_id != WorkingIndex) + { + updateEng.getInvRow(psiM, WorkingIndex, invRow); + invRow_id = WorkingIndex; + } + curRatio = simd::dot(invRow.data(), psiV.data(), invRow.size()); RatioTimer.stop(); return curRatio; } diff --git a/src/QMCWaveFunctions/Fermion/DiracDeterminant.h b/src/QMCWaveFunctions/Fermion/DiracDeterminant.h index f6ce51c65d..147d26d5b2 100644 --- a/src/QMCWaveFunctions/Fermion/DiracDeterminant.h +++ b/src/QMCWaveFunctions/Fermion/DiracDeterminant.h @@ -183,6 +183,16 @@ class DiracDeterminant: public DiracDeterminantBase DiracMatrix detEng; DelayedUpdate updateEng; + /// the row of up-to-date inverse matrix + ValueVector_t invRow; + + /** row id correspond to the up-to-date invRow. [0 norb), invRow is ready; -1, invRow is not valid. + * This id is set after calling getInvRow indicating invRow has been prepared for the invRow_id row + * ratioGrad checks if invRow_id is consistent. If not, invRow needs to be recomputed. + * acceptMove and completeUpdates mark invRow invalid by setting invRow_id to -1 + */ + int invRow_id; + ValueType curRatio,cumRatio; ParticleSet::SingleParticleValue_t *FirstAddressOfG; ParticleSet::SingleParticleValue_t *LastAddressOfG; diff --git a/src/QMCWaveFunctions/tests/test_dirac_matrix.cpp b/src/QMCWaveFunctions/tests/test_dirac_matrix.cpp index 0b3255ce2d..828a196640 100644 --- a/src/QMCWaveFunctions/tests/test_dirac_matrix.cpp +++ b/src/QMCWaveFunctions/tests/test_dirac_matrix.cpp @@ -121,13 +121,13 @@ TEST_CASE("DiracMatrix_update_row", "[wavefunction][fermion]") dm.invert(a,false); // new row - Vector v; - v.resize(3); + Vector v(3), invRow(3); v[0] = 1.9; v[1] = 2.0; v[2] = 3.1; - ValueType det_ratio1 = updateEng.ratio(a,0,v); + updateEng.getInvRow(a,0,invRow); + ValueType det_ratio1 = simd::dot(v.data(), invRow.data(), invRow.size()); ValueType det_ratio = 0.178276269185; REQUIRE(det_ratio1 == ValueApprox(det_ratio)); From 5380ee8ac8ff1da5e1316755bc8f43953d28fe6a Mon Sep 17 00:00:00 2001 From: Ye Luo Date: Tue, 26 Feb 2019 00:55:25 -0600 Subject: [PATCH 2/4] fix TM1/3 with DU case. --- src/QMCWaveFunctions/Fermion/DiracDeterminant.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/QMCWaveFunctions/Fermion/DiracDeterminant.cpp b/src/QMCWaveFunctions/Fermion/DiracDeterminant.cpp index bf535d97a6..d405b41b3c 100644 --- a/src/QMCWaveFunctions/Fermion/DiracDeterminant.cpp +++ b/src/QMCWaveFunctions/Fermion/DiracDeterminant.cpp @@ -117,8 +117,8 @@ DiracDeterminant::evalGrad(ParticleSet& P, int iat) { const int WorkingIndex = iat-FirstIndex; RatioTimer.start(); - updateEng.getInvRow(psiM, WorkingIndex, invRow); invRow_id = WorkingIndex; + updateEng.getInvRow(psiM, WorkingIndex, invRow); GradType g = simd::dot(invRow.data(), dpsiM[WorkingIndex], invRow.size()); RatioTimer.stop(); return g; @@ -139,8 +139,8 @@ DiracDeterminant::ratioGrad(ParticleSet& P, int iat, GradType& grad_iat) // This is not a safety check. Some code paths do call ratioGrad without calling evalGrad first. if(invRow_id != WorkingIndex) { - updateEng.getInvRow(psiM, WorkingIndex, invRow); invRow_id = WorkingIndex; + updateEng.getInvRow(psiM, WorkingIndex, invRow); } curRatio = simd::dot(invRow.data(), psiV.data(), invRow.size()); grad_iat += ((RealType)1.0/curRatio) * simd::dot(invRow.data(), dpsiV.data(), invRow.size()); @@ -283,8 +283,8 @@ DiracDeterminant::ValueType DiracDeterminant::ratio(ParticleSet& P, int iat) // This is intended to save redundant compuation in TM1 and TM3 if(invRow_id != WorkingIndex) { - updateEng.getInvRow(psiM, WorkingIndex, invRow); invRow_id = WorkingIndex; + updateEng.getInvRow(psiM, WorkingIndex, invRow); } curRatio = simd::dot(invRow.data(), psiV.data(), invRow.size()); RatioTimer.stop(); @@ -293,9 +293,11 @@ DiracDeterminant::ValueType DiracDeterminant::ratio(ParticleSet& P, int iat) void DiracDeterminant::evaluateRatios(VirtualParticleSet& VP, std::vector& ratios) { - ValueVector_t psiM_row(psiM[VP.refPtcl-FirstIndex], psiM.cols()); SPOVTimer.start(); - Phi->evaluateDetRatios(VP, psiV, psiM_row, ratios); + const int WorkingIndex = VP.refPtcl-FirstIndex; + invRow_id = WorkingIndex; + updateEng.getInvRow(psiM, WorkingIndex, invRow); + Phi->evaluateDetRatios(VP, psiV, invRow, ratios); SPOVTimer.stop(); } From e61659bb7e642e0134fb76a8de4c4fb43aae08b1 Mon Sep 17 00:00:00 2001 From: Ye Luo Date: Tue, 26 Feb 2019 10:49:55 -0600 Subject: [PATCH 3/4] Minor change. --- src/QMCWaveFunctions/Fermion/DelayedUpdate.h | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/QMCWaveFunctions/Fermion/DelayedUpdate.h b/src/QMCWaveFunctions/Fermion/DelayedUpdate.h index f72eaa3631..4685ae3b40 100644 --- a/src/QMCWaveFunctions/Fermion/DelayedUpdate.h +++ b/src/QMCWaveFunctions/Fermion/DelayedUpdate.h @@ -76,13 +76,12 @@ namespace qmcplusplus { } const T cone(1); const T czero(0); - const T* AinvRow = Ainv[rowchanged]; const int norb = Ainv.rows(); const int lda_Binv = Binv.cols(); - // save AinvRow to new_AinvRow - std::copy_n(AinvRow, norb, invRow.data()); - // multiply V (NxK) Binv(KxK) U(KxN) AinvRow right to the left - BLAS::gemv('T', norb, delay_count, cone, U.data(), norb, AinvRow, 1, czero, p.data(), 1); + // save Ainv[rowchanged] to invRow + std::copy_n(Ainv[rowchanged], norb, invRow.data()); + // multiply V (NxK) Binv(KxK) U(KxN) invRow right to the left + BLAS::gemv('T', norb, delay_count, cone, U.data(), norb, invRow.data(), 1, czero, p.data(), 1); BLAS::gemv('N', delay_count, delay_count, cone, Binv.data(), lda_Binv, p.data(), 1, czero, Binv[delay_count], 1); BLAS::gemv('N', norb, delay_count, -cone, V.data(), norb, Binv[delay_count], 1, cone, invRow.data(), 1); } From f2fefd4ec5bd7432bfe64c154458d9dd5b0acf2f Mon Sep 17 00:00:00 2001 From: Ye Luo Date: Wed, 27 Feb 2019 12:16:48 -0600 Subject: [PATCH 4/4] More clear comment. --- src/QMCWaveFunctions/Fermion/DiracDeterminant.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/QMCWaveFunctions/Fermion/DiracDeterminant.cpp b/src/QMCWaveFunctions/Fermion/DiracDeterminant.cpp index d405b41b3c..6575265cf7 100644 --- a/src/QMCWaveFunctions/Fermion/DiracDeterminant.cpp +++ b/src/QMCWaveFunctions/Fermion/DiracDeterminant.cpp @@ -135,8 +135,9 @@ DiracDeterminant::ratioGrad(ParticleSet& P, int iat, GradType& grad_iat) UpdateMode=ORB_PBYP_PARTIAL; GradType rv; - // check invRow_id against WorkingIndex to ensure getInvRow() called before ratioGrad() - // This is not a safety check. Some code paths do call ratioGrad without calling evalGrad first. + // This is an optimization. + // check invRow_id against WorkingIndex to see if getInvRow() has been called already + // Some code paths call evalGrad before calling ratioGrad. if(invRow_id != WorkingIndex) { invRow_id = WorkingIndex; @@ -264,6 +265,8 @@ void DiracDeterminant::copyFromBuffer(ParticleSet& P, WFBufferType& buf) d2psiM.attachReference(buf.lendReference(d2psiM.size())); buf.get(LogValue); buf.get(PhaseValue); + // start with invRow labelled invalid + invRow_id = -1; BufferTimer.stop(); } @@ -279,6 +282,7 @@ DiracDeterminant::ValueType DiracDeterminant::ratio(ParticleSet& P, int iat) Phi->evaluate(P, iat, psiV); SPOVTimer.stop(); RatioTimer.start(); + // This is an optimization. // check invRow_id against WorkingIndex to see if getInvRow() has been called // This is intended to save redundant compuation in TM1 and TM3 if(invRow_id != WorkingIndex)