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

Specify ip as OMP Shared Variable in CalHam #3

Merged
merged 1 commit into from
May 21, 2019

Conversation

xrq-phys
Copy link
Collaborator

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?

Const arguments should be specified as shared in openmp. This mistake was ignored by all previous compilers until GCC 9.
@tmisawa tmisawa merged commit 3612984 into issp-center-dev:master May 21, 2019
@tmisawa
Copy link
Collaborator

tmisawa commented May 21, 2019

@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 xrq-phys deleted the omp-fix-rigorous branch May 21, 2019 02:58
@xrq-phys
Copy link
Collaborator Author

xrq-phys commented May 21, 2019 via email

@xrq-phys
Copy link
Collaborator Author

xrq-phys commented Jul 14, 2019

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
Copy link
Collaborator Author

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

@tmisawa tmisawa mentioned this pull request Oct 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants