Skip to content

{chem}[intel/2017b] Dalton 2016#5808

Merged
boegel merged 7 commits intoeasybuilders:developfrom
hajgato:dalton
Oct 12, 2020
Merged

{chem}[intel/2017b] Dalton 2016#5808
boegel merged 7 commits intoeasybuilders:developfrom
hajgato:dalton

Conversation

@hajgato
Copy link
Copy Markdown
Collaborator

@hajgato hajgato commented Feb 10, 2018

All (non-benchmark) test jobs were passed
Slightly different approach compared to #5778

@boegel
Copy link
Copy Markdown
Member

boegel commented Feb 11, 2018

@akesandgren Are you up for giving this a quick review?

@hajgato Can you point out the differences with #5778?

configopts = '-DCMAKE_BUILD_TYPE=RELEASE '
configopts += '-DENABLE_MPI=ON '
configopts += '-DENABLE_OMP=ON '
configopts += '-DENABLE_64BIT_INTEGERS=ON '
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd avoid using 64bit ints for the uploaded recipy.
It's not compatible with most scalapack/blacs/lapack/blas builds.

configopts += '-DENABLE_AUTO_BLAS=OFF '
configopts += '-DENABLE_AUTO_LAPACK=OFF '
configopts += '-DEXPLICIT_BLAS_LIB="$LIBBLAS" '
configopts += '-DEXPLICIT_LAPACK_LIB="$LIBLAPACK" '
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You probably need
configopts += '-DBLACS_IMPLEMENTATION=intelmpi '
configopts += '-DENABLE_SCALAPACK=ON'

Not using scalapack will make it a fair bit slower.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

If you check the CMake files, you will see that ScaLAPACK/BLACS only works with MKL. Meanwhile enable 64 bit integers would work with MKL, ACLM out-of-the box, and you can easily make 64 bit integer version of OPENBLAS , and ATLAS. So I think the same should apply to ENABLE_64BIT_INTEGERS=ON and ENABLE_SCALAPACK=ON

@hajgato
Copy link
Copy Markdown
Collaborator Author

hajgato commented Feb 12, 2018

Differences compared to #5778

  • 64-bit integers (I would stick to them, will give a comment only use in case of MKL)
  • Dalton compiler flags are controlled by EasyBuild instead using hard-coded optimization flag -O3 (although extensions still use hard-coded ones)
  • All broken test jobs are fixed (switching back to lowopt -O1) when it is needed.

I will fix ScaLAPACK as well, but I have to patch CMake files, because by default (see switches above) will use the Fortran bundled ScsLAPACK instead of the MKL one. Anyway, its only beneficial to use for internode MPI jobs.

@hajgato
Copy link
Copy Markdown
Collaborator Author

hajgato commented Feb 12, 2018

I was wrong, CMake will actually use MKL from the imkl not the bundled one.

@hajgato
Copy link
Copy Markdown
Collaborator Author

hajgato commented Feb 12, 2018

@akesandgren @boegel ^

@migueldiascosta
Copy link
Copy Markdown
Member

@akesandgren are you ok with the scalapack changes and added comment on 64bit integers? (or maybe this one should have an _i8 versionsuffix?)

if(CMAKE_Fortran_COMPILER_ID MATCHES Intel)
add_definitions(-DVAR_IFORT)
- set(CMAKE_Fortran_FLAGS "-fpp -assume byterecl -DVAR_IFORT")
+ set(CMAKE_Fortran_FLAGS "${CMAKE_Fortran_FLAGS} -fpp -DVAR_IFORT")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missed this one last time around.
I'm fairly certain you have to keep -assume byterecl for Intel fortran

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@akesandgren Not necessarily, the DIRECT_IO_FACTOR can be 1 or 4 with or without byterecl. There is a setting for Dalton I forget where it is, i vaguely remember that. Anyway, if you use the wring model, `Dalton will not work at all.

@akesandgren
Copy link
Copy Markdown
Contributor

It might be worth it to have the -i8 build have a versionsuffix specifying this, but since -i8 is the preferred way for dalton, one might set versionsuffix -i4 on the non-i8 builds...
Don't care one way or the other, as long as they are clearly distinct.

And -i8/4 is probably not the most informative suffix, but I don't know of any better at the moment.

At the very least one should add some explanation to the module file.

We never bothered building it with -i8 since there was never a reason to do it for our users.

Other than that I have no objections to it.


configopts = '-DCMAKE_BUILD_TYPE=RELEASE '
configopts += '-DENABLE_MPI=ON '
configopts += '-DENABLE_OMP=ON '
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

And if you enable OMP you should probably add openmp: True in toolchainopts?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done

@boegel boegel added the new label Oct 12, 2020
@boegel boegel added this to the 4.x milestone Oct 12, 2020
@boegel
Copy link
Copy Markdown
Member

boegel commented Oct 12, 2020

Test report by @boegel
SUCCESS
Build succeeded for 1 out of 1 (1 easyconfigs in this PR)
node2447.golett.os - Linux centos linux 7.8.2003, x86_64, Intel(R) Xeon(R) CPU E5-2680 v3 @ 2.50GHz (haswell), Python 2.7.5
See https://gist.github.com/e84419eceb392362d2df5b8f6673914a for a full test report.

@boegel boegel changed the title {chem}[intel/2017b] Dalton 2016 (REVIEW) {chem}[intel/2017b] Dalton 2016 Oct 12, 2020
@easybuilders easybuilders deleted a comment from boegelbot Oct 12, 2020
@boegel
Copy link
Copy Markdown
Member

boegel commented Oct 12, 2020

Test report by @boegel
SUCCESS
Build succeeded for 1 out of 1 (1 easyconfigs in this PR)
node3404.kirlia.os - Linux centos linux 7.8.2003, x86_64, Intel(R) Xeon(R) Gold 6240 CPU @ 2.60GHz (cascadelake), Python 2.7.5
See https://gist.github.com/09be0a71edb08611cacaec59675d0625 for a full test report.

@boegel
Copy link
Copy Markdown
Member

boegel commented Oct 12, 2020

Test report by @boegel
SUCCESS
Build succeeded for 1 out of 1 (1 easyconfigs in this PR)
node3134.skitty.os - Linux centos linux 7.8.2003, x86_64, Intel(R) Xeon(R) Gold 6140 CPU @ 2.30GHz, Python 3.6.8
See https://gist.github.com/20c15b4cc1a7e129d2715394400c7973 for a full test report.

@boegel boegel dismissed akesandgren’s stale review October 12, 2020 11:56

all requested changes made

@boegel
Copy link
Copy Markdown
Member

boegel commented Oct 12, 2020

Going in, thanks @hajgato!

@boegel boegel merged commit fefd03f into easybuilders:develop Oct 12, 2020
@boegel boegel removed this from the 4.x milestone Oct 14, 2020
@boegel boegel added this to the next release (4.3.1) milestone Oct 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants