Skip to content

disable USER-INTEL package in LAMMPS easyconfigs using intel/2019b, since it results in an installation that produces incorrect results#10418

Merged
ocaisa merged 3 commits intoeasybuilders:developfrom
boegel:20200416175514_new_pr_LAMMPS3Mar2020
Apr 22, 2020
Merged

disable USER-INTEL package in LAMMPS easyconfigs using intel/2019b, since it results in an installation that produces incorrect results#10418
ocaisa merged 3 commits intoeasybuilders:developfrom
boegel:20200416175514_new_pr_LAMMPS3Mar2020

Conversation

@boegel
Copy link
Copy Markdown
Member

@boegel boegel commented Apr 16, 2020 •
edited
Loading

(created using eb --new-pr)

I have access to a small test case that catches the problem, I'm looking into whether we can include it as a sanity check (it's short enough, but I'm not sure yet whether it can be made public)

requires easybuilders/easybuild-easyblocks#2031 to correctly disable the -DPKG_USER-INTEL=on that is added by default by the LAMMPS easyblock based on configopts specified in easyconfigs...

edit: requires easybuilders/easybuild-framework#3288 to ensure full sanity check is run

@boegel
disable USER-INTEL package in LAMMPS easyconfigs using intel/2019b, s…
e513938 
…ince it results in an installation that produces incorrect results
@boegel boegel added the bug fix label Apr 16, 2020
@boegel boegel added this to the next release (4.2.1?) milestone Apr 16, 2020
@boegel boegel mentioned this pull request Apr 16, 2020
Merged
@boegel
Copy link
Copy Markdown
Member Author

boegel commented Apr 17, 2020

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

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Apr 17, 2020

Test report by @boegel
SUCCESS
Build succeeded for 4 out of 4 (4 easyconfigs in this PR)
generoso - Linux centos linux 7.6.1810, Intel(R) Xeon(R) CPU E5-2660 v4 @ 2.00GHz, Python 3.6.8
See https://gist.github.com/930a35bceb0b6f21b369f6a9786f9def for a full test report.

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Apr 17, 2020

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

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Apr 17, 2020

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

@ocaisa
Copy link
Copy Markdown
Member

ocaisa commented Apr 17, 2020

This does what is intended but also overrides all the existing sanity_check_commands in the easyblock, as opposed to adding to them.

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Apr 17, 2020

This does what is intended but also overrides all the existing sanity_check_commands in the easyblock, as opposed to adding to them.

Oh, nice catch, that's indeed not the intention here...

This has come up before, I think it's time we add support for something like enhance_sanity_check = True in framework, I'll look into it...

@lexming
Copy link
Copy Markdown
Contributor

lexming commented Apr 17, 2020

Test report by @lexming
FAILED
Build succeeded for 12 out of 14 (4 easyconfigs in this PR)
node150.hydra.os - Linux centos linux 7.7.1908, Intel(R) Xeon(R) CPU E5-2680 v2 @ 2.80GHz, Python 2.7.5
See https://gist.github.com/a64db2d5cd92282814ea4d6f7b66c988 for a full test report.

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Apr 17, 2020

@lexming Looks like you were testing without the fixed LAMMPS easyblock from easybuilders/easybuild-easyblocks#2031, is that correct?

@lexming
Copy link
Copy Markdown
Contributor

lexming commented Apr 17, 2020

@boegel you are right, I cannot read 🙂

@lexming
Copy link
Copy Markdown
Contributor

lexming commented Apr 17, 2020

Test report by @lexming
SUCCESS
Build succeeded for 4 out of 4 (4 easyconfigs in this PR)
node150.hydra.os - Linux centos linux 7.7.1908, Intel(R) Xeon(R) CPU E5-2680 v2 @ 2.80GHz, Python 2.7.5
See https://gist.github.com/636d06f49cc624247954ef21c0bd7d5e for a full test report.

@lexming
Copy link
Copy Markdown
Contributor

lexming commented Apr 17, 2020

@boegel I got the following from the sanity check script in v3Mar2020

Yaff energy
-83608.90672145087
valence: 789.127 kJ/mol
pair_ei: -31825.911 kJ/mol
ewald_reci: 154.317 kJ/mol
ewald_cor: -52763.514 kJ/mol
ewald_neut: -0.000 kJ/mol
pair_mm3: 37.075 kJ/mol
LAMMPS energy
-83609.2416305773
valence: 789.127 kJ/mol
lammps: -237558.856 kJ/mol
pair_ei: 153160.487 kJ/mol

I guess that as long as the total energy value from yaff is close to LAMMPS the results are reliable. Is that so? should I check anything else apart from the total energy?

@boegel boegel mentioned this pull request Apr 17, 2020
Merged
@boegel
Copy link
Copy Markdown
Member Author

boegel commented Apr 17, 2020

@lexming That's right, the energies have to be close enough. If they're not, then the script fails (see comments on top of the script).

So: lgtm!

Don't merge this PR yet though, see @ocaisa's remark.

I would like to see easybuilders/easybuild-framework#3288 merged first, so I can add enhanced_sanity_check = True in these easyconfigs, to avoid skipping the sanity check commands that are run by the LAMMPS easyblock...

@ocaisa
Copy link
Copy Markdown
Member

ocaisa commented Apr 21, 2020

@boegel Now that easybuilders/easybuild-framework#3288 is merged, this needs an update.

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Apr 22, 2020

@ocaisa enhance_sanity_check = True added, new test reports coming in...

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Apr 22, 2020

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

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Apr 22, 2020

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

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Apr 22, 2020

Test report by @boegel
SUCCESS
Build succeeded for 4 out of 4 (4 easyconfigs in this PR)
generoso-3 - Linux centos linux 7.6.1810, x86_64, Intel(R) Xeon(R) CPU E5-2660 v4 @ 2.00GHz, Python 3.6.8
See https://gist.github.com/d517619bce2379b766138ab0871264bc for a full test report.

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Apr 22, 2020

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

@ocaisa
Copy link
Copy Markdown
Member

ocaisa commented Apr 22, 2020

Going in, thanks @boegel !

@ocaisa ocaisa merged commit 8228c18 into easybuilders:develop Apr 22, 2020
@lexming
Copy link
Copy Markdown
Contributor

lexming commented Apr 22, 2020

Test report by @lexming
SUCCESS
Build succeeded for 4 out of 4 (4 easyconfigs in this PR)
node115.hydra.os - Linux centos linux 7.7.1908, x86_64, Intel(R) Xeon(R) CPU E5-2680 v2 @ 2.80GHz, Python 2.7.5
See https://gist.github.com/e5865af140aff883eb377e9d927c81e8 for a full test report.

@boegel boegel deleted the 20200416175514_new_pr_LAMMPS3Mar2020 branch April 22, 2020 16:34
@boegel boegel mentioned this pull request Apr 29, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ocaisa ocaisa ocaisa approved these changes

Assignees

No one assigned

Labels

bug fix

Projects

None yet

Milestone

4.2.1

Development

Successfully merging this pull request may close these issues.

3 participants

@boegel @ocaisa @lexming