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

{chem}[foss/2020b] LAMMPS v29Sep2021 #14653

Closed

Conversation

arkdavy
Copy link

@arkdavy arkdavy commented Jan 4, 2022

(created using eb --new-pr)
requires easybuilders/easybuild-easyblocks#2213

…Sep2021-foss-2020b-kokkos.eb, LAMMPS-29Sep2021-foss-2020b-kokkos-omp.eb, molmod-1.4.5-foss-2020b-Python-3.8.6.eb, yaff-1.6.0-foss-2020b-Python-3.8.6.eb and patches: LAMMPS-29Sep2021-fixdoc.patch
@migueldiascosta
Copy link
Member

thanks for your first contribution @arkdavy! good to see you're already using eb's github integration!

please note that in the 2020b toolchains and more recent, we're no longer using the -Python-%(pyver)s versionsuffix if the python version is the default one for that toolchain, which is the case here

additionally, there has been a package reorganization in LAMMPS, the only remaining USER package is USER-MISC (lammps/lammps#2818), so I'm not sure what's happening to your user_packages. See also #14414 if you haven't already?

@migueldiascosta migueldiascosta added this to the 4.x milestone Jan 4, 2022
@arkdavy
Copy link
Author

arkdavy commented Jan 4, 2022

Dear @migueldiascosta , thanks for viewing my PR, I am happy to contribute! I have many easyconfigs, I just need to learn how to submit PRs properly.

Regarding the user packages, there is a required by easyblock user_packages list, and that was filled with packages which were called USER-* before, but now those have different names, as you pointed out. But indeed, combining it into one list as in #14414 should be a cleaner solution. The LAMMPS package lists are almost identical between my and #14414 PR.

I had to delete yaff and molmod easyconfigs from this PR, because these were found in the development branch after removing suffixes (which means that molmod will be deleted from #14414 as well). That also caused a deletion of h5py-2.10.0-foss-2020b, which was found inside the yaff-1.6.0-foss-2020b.eb easyconfig.

Then, I have managed to compile documentation using the submitted patch and the Doxygen builddependency

Finally, #14414 is a fosscuda compilation, I guess it will be useful to have the CPU-LAMMPS as well.

@migueldiascosta migueldiascosta changed the title {chem}[foss/2020b] h5py v2.10.0, LAMMPS v29Sep2021, molmod v1.4.5, ... w/ Python 3.8.6 {chem}[foss/2020b] LAMMPS v29Sep2021 Jan 5, 2022
@migueldiascosta
Copy link
Member

@arkdavy without a doubt, this is a useful contribution, in addition to the fosscuda one

the easyblock does still need work, which will be a requirement for merging this PR, but we can still work on this in the meantime

yes, #14523 added yaff, with a bundled h5py version 2, and molmod for this toolchain, so that's good

is having separate variants with and without openmp really necessary/valuable in this case?

@arkdavy
Copy link
Author

arkdavy commented Jan 5, 2022

Dear Miguel, I was not sure which one to choose. The easyblock has OpenMP in Kokkos as a default. Compiling with the serial backend is suggested by the code when running with one OpenMP thread. My tests were showing that having the serial backend in Kokkos gives a minor performance gain in pure MPI calculations (in simple tests). In spite of this, maybe a better option would be to use the default settings and to leave the instructions on how to compile with serial by either turning on the corresponding flag (as I have done) or by switching off the OpenMP in Kokkos. Therefore, I have deleted the -omp.eb file and added the corresponding instructions.

Another problem is that VTK package is not compiling anymore. It wasn't actually selected in my previous tests when some packages were in the user_pacakges list. After looking at the easyblock, I see that it adds 'USER-' to package names inside user_pacakges list, and apparently, this version of LAMMPS has limited support of -DUSER-PACKAGE=on/off legacy format in the configuration step. Therefore VTK is excluded.

Moreover, I have tested the #14414 easyconfig, and VTK is not selected for compilation there, although it is inside the correct general_packages list. After double-checking, I see that VTK is not in the package list (i.e., not in general_packages), but the VTK library is in the dependency list, and one could delete it from #14414.

@arkdavy
Copy link
Author

arkdavy commented Jan 6, 2022

I have added checksum, but another error is about missing yaff, which does exist in the develop branch. Can/should we test against that one?

@migueldiascosta
Copy link
Member

@arkdavy your commit f5d8fe8 is actually deleting the existing easyconfig, you just need to add it back

@arkdavy
Copy link
Author

arkdavy commented Jan 6, 2022

It's interesting, I thought I was deleting the file I was uploading. This is apparently not the case when such a file exists in the develop branch.

@arkdavy
Copy link
Author

arkdavy commented Jan 6, 2022

I guess I am not able to do anything with the last test error, it somehow does not like several existing PLUMED versions with the same toolchain (foss-2020b).

@migueldiascosta
Copy link
Member

w.r.t. PLUMED, there are two options:

this is because we try to only use a single version of a dependency per toolchain version (the check is only for the use as a dependency, you're right that there are already different versions of PLUMED with this toolchain version), but even then there are exceptions, as you can see

(in this particular case, it may make more sense to say that it's CP2K that requires an older version, in old_dep_versions)

@arkdavy arkdavy force-pushed the 20220104130926_new_pr_h5py2100 branch from b84967b to 1abed00 Compare January 8, 2022 00:24
@arkdavy
Copy link
Author

arkdavy commented Jan 8, 2022

Ok, I took a simpler path using 2.6.2 version of PLUMED. In principle, both of them can be regarded to 2020b toolchain generation since 2.6.2 was released on 26 Oct 2020, 2.7.0 was released on 23 Dec 2020. There are many CP2K configs with this PLUMED-2.6.2, therefore, it must be well-tested.

@easybuilders easybuilders deleted a comment from boegelbot Jan 10, 2022
@migueldiascosta
Copy link
Member

@boegelbot please test @ generoso
EB_ARGS="--include-easyblocks-from-pr 2213"

@easybuilders easybuilders deleted a comment from boegelbot Jan 10, 2022
@boegelbot
Copy link
Collaborator

@migueldiascosta: Request for testing this PR well received on login1

PR test command 'EB_PR=14653 EB_ARGS="--include-easyblocks-from-pr 2213" /opt/software/slurm/bin/sbatch --job-name test_PR_14653 --ntasks=4 ~/boegelbot/eb_from_pr_upload_generoso.sh' executed!

  • exit code: 0
  • output:
Submitted batch job 7676

Test results coming soon (I hope)...

- notification for comment with ID 1008508191 processed

Message to humans: this is just bookkeeping information for me,
it is of no use to you (unless you think I have a bug, which I don't).

@boegelbot
Copy link
Collaborator

Test report by @boegelbot
Using easyblocks from PR(s) easybuilders/easybuild-easyblocks#2213
SUCCESS
Build succeeded for 1 out of 1 (1 easyconfigs in total)
cns1 - Linux rocky linux 8.4, x86_64, Intel(R) Xeon(R) CPU E5-2667 v3 @ 3.20GHz (haswell), Python 3.6.8
See https://gist.github.com/837c989eb48db7252ba198decfa43015 for a full test report.

@migueldiascosta
Copy link
Member

this looks good to me, but it requires easybuilders/easybuild-easyblocks#2213, and that will still need to be made backwards compatible, so it may take a while to merge

@genio0815
Copy link
Contributor

Hi all,
also started working on this...the issue with VTK is that a version < 9 is needed acc
https://github.com/lammps/lammps/blob/stable_29Sep2021/src/VTK/dump_vtk.cpp#L98

I already have a working VTK-8.2.0-foss-2020b version (which requires a patch to compile with gcc 10.2.0) which I'd be happy to contribute unless the "single version of a dependency per toolchain version" consideration doesn't kick in (there is already VTK-9.0.1-foss-2020b).

@arkdavy
Copy link
Author

arkdavy commented Jan 31, 2022

Would you like to attach the file to this PR, or we just add VTK as a dependency here?

@genio0815
Copy link
Contributor

genio0815 commented Feb 5, 2022

Attaching here or new PR, both fine, whatever fits best... The VTK 8.2.0 module itself works fine, but with lammps does not compile due to lammps/lammps#2998 and https://matsci.org/t/vtk-package-source-files-seem-to-have-had-syntax-errors-introduced/38775 but should be fixed by their latest patches
https://github.com/lammps/lammps/releases/download/stable_29Sep2021/bugfixes_for_stable_29Sep2021.patch.gz
Thus to use the VTK package, pls add mentioned patch

@sassy-crick
Copy link
Collaborator

I got a request for LAMMPS v29Sep2021 as well. Given some time has passed, and there are a few PRs open for LAMMPS like for exampe PR#14815, I was wondering if any work has been done here since 5.2. Happy to contribute if that helps.
Thanks

@arkdavy
Copy link
Author

arkdavy commented May 9, 2022

Sorry for the late reply. I think I have managed to push a small easyblock update easybuilders/easybuild-easyblocks#2213, which will be backwards compatible, but that has to be checked before we can progress here.

@sassy-crick
Copy link
Collaborator

From what I can see, PR #2213 has been merged and I only noticed you mentioning this PR there but no new commits. Maybe it is best to open a new PR for the changes you are proposing so there is a clear history?

@arkdavy
Copy link
Author

arkdavy commented May 9, 2022

Oh, sorry, I have made a mess mentioning that number because this must be a reference to PR for easyblock (I am updating my previous comment accordingly): easybuilders/easybuild-easyblocks#2213

@ocaisa
Copy link
Member

ocaisa commented Jul 20, 2022

@arkdavy Do you want to keep this PR alive? If so, can you test it with the updated easybuilders/easybuild-easyblocks#2213

@arkdavy
Copy link
Author

arkdavy commented Jul 21, 2022

Hi Let's keep it alive for a while, didn't have time to fix/test it, but I would like to complete it definitely

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.

6 participants