issp-center-dev / mVMC

A numerical solver package for a wide range of quantum lattice models based on many-variable Variational Monte Carlo method
http://www.pasums.issp.u-tokyo.ac.jp/mvmc/en/
GNU General Public License v3.0
59 stars 19 forks source link

Specify ip as OMP Shared Variable in CalHam #3

Closed xrq-phys closed 5 years ago

xrq-phys commented 5 years ago

Hello. This is Ruqing Xu.

In the #pragma omp parallel section in CalculateHamiltonian_real and CalculateHamiltonian, constant argument ip was not set as shared, while default is none (for rigorous control of omp's behavior maybe?). All previous compilers has ignored this and shared ip implicitly, but the latest GCC 9 is reporting an error. Hence it should get fixed I think?

tmisawa commented 5 years ago

@xrq-phys Because ip is defined as const in CalculateHamiltonian, many compliers allow the implicit definition of ip. You changes are reasonable and more rigorous, then we merged your pull request.

thanks !

xrq-phys commented 5 years ago

Thanks for taking care of this pull request!

On Tue, May 21, 2019 at 11:54 AM Takahiro Misawa notifications@github.com wrote:

@xrq-phys https://github.com/xrq-phys Because ip is defined as const in CalculateHamiltonian, many compliers allow the implicit definition of ip. You changes are reasonable and more rigorous, then we merged your pull request.

thanks !

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/issp-center-dev/mVMC/pull/3?email_source=notifications&email_token=AB4GUGTBIQWE72ZVCPNFKEDPWNP5TA5CNFSM4HN2Q2JKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODV2THJA#issuecomment-494220196, or mute the thread https://github.com/notifications/unsubscribe-auth/AB4GUGWBUAGXCXG45JQ7KYDPWNP5TANCNFSM4HN2Q2JA .

xrq-phys commented 5 years ago

I think I should mention additionally that some versions of GCC4 (4.7.2, 4.2, etc.) seems forbidding explicitly specifying ip as shared (ICC/ICPC doesn't have this problem). Though rigorously speaking ip should always be specified as shared (according to OpenMP 4.0 and OpenMP 5.0), care might be taken as GCC4 (OpenMP 3.1) is still widely used, I guess.

Error message thrown could be something like: error: 'ip' is predetermined 'shared' for 'shared' Ref: https://stackoverflow.com/questions/13199398/openmp-predetermined-shared-for-shared Ref: https://www.gnu.org/software/gcc/gcc-9/porting_to.html

xrq-phys commented 5 years ago

A supposed fix could be something like this in CalculateHamiltonian_real and CalculateHamiltonian:

#ifdef _OPENMP
#if _OPENMP > 201307
  #pragma omp parallel default(none) \
    private(myEleIdx,myEleNum,myProjCntNew,myBuffer,myEnergy, idx, ri, rj, rk, rl, s, t) \
    firstprivate(Nsize, Nsite2, NProj, NQPFull, NCoulombIntra, CoulombIntra, ParaCoulombIntra, \
    NCoulombInter, CoulombInter, ParaCoulombInter, NHundCoupling, HundCoupling, ParaHundCoupling, \
    NTransfer, Transfer, ParaTransfer, NPairHopping, PairHopping, ParaPairHopping, \
    NExchangeCoupling, ExchangeCoupling, ParaExchangeCoupling, NInterAll, InterAll, ParaInterAll, n0, n1) \
    shared(eleCfg, eleProjCnt, eleIdx, eleNum, ip) reduction(+:e)
#else
  #pragma omp parallel default(none) \
    private(myEleIdx,myEleNum,myProjCntNew,myBuffer,myEnergy, idx, ri, rj, rk, rl, s, t) \
    firstprivate(Nsize, Nsite2, NProj, NQPFull, NCoulombIntra, CoulombIntra, ParaCoulombIntra, \
    NCoulombInter, CoulombInter, ParaCoulombInter, NHundCoupling, HundCoupling, ParaHundCoupling, \
    NTransfer, Transfer, ParaTransfer, NPairHopping, PairHopping, ParaPairHopping, \
    NExchangeCoupling, ExchangeCoupling, ParaExchangeCoupling, NInterAll, InterAll, ParaInterAll, n0, n1) \
    shared(eleCfg, eleProjCnt, eleIdx, eleNum) reduction(+:e)
#endif
#endif