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}[intel/2018b] PSI4 1.2.1 (REVIEW) #7109

Merged
merged 9 commits into from
Nov 14, 2018
Merged

Conversation

hajgato
Copy link
Collaborator

@hajgato hajgato commented Nov 2, 2018

needs easybuilders/easybuild-easyblocks#1568
PCMSolver (tried 1.1.12, and 1.2.1 versions) cannot compiled with intel compilers, even using -O0 and generic architecture.
(see PCMSolver/pcmsolver#159)

@boegel boegel added the update label Nov 3, 2018
# Tests are failing with pytest 3.9.2
('pytest', '3.8.2', versionsuffix),
('networkx', '2.2', versionsuffix),
('deepdiff', '3.3.0', versionsuffix),
Copy link
Member

Choose a reason for hiding this comment

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

@hajgato Please include a commented out entry for PCMSolver, and explain why it's excluded for now.

Also, why are libxc, Libint & co not included as deps (given the updates to the easyblock in easybuilders/easybuild-easyblocks#1568)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@boegel
I did not include libxc and Libint, because those should be installed with CMake, with specific options. The current libxc and Libint are built with configure, not with CMake, plus PSI4 have specific requirements for Libint with a unified libderiv includedir, and with a different scheme of libint-max-am and libderiv-max-am1. Further compilations, that with the CMake cannot enable r12 options for Libint. As a consequence, I should use a specific version of Libint for PSI4, (which would be not compatible with a possible future version of CP2K and a new version of libxc which was built with CMake instead of the existing configure. The moment I faced this problem, the psi easyblock was already done, and I did not delete that part.
If you wish, I can make the PSI4 specific Libint and libxc easyconfigs with the -PSI4 suffix, and/or I can delete the Libint and libxc parts from the psi easyblock.

Copy link
Member

Choose a reason for hiding this comment

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

So what happens if we don't provide libxc and Libint as dependencies ourselves? Are they downloaded on the fly during the installation of PSI4? If so, that's a good reason to include them as dependencies instead imho, even if they are very specific to PSI4 (but we should try and make them a bit more generic if possible by not using PSI in the versionsuffix).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@boegel
Yes, they downloaded and built automatically. I am not sure in case of libxc that it can be replaced by a generic one, as it will clash with CP2K (see for specific include dir). Maybe in the case of Libint we can make a more generic easyconfig, but then we have to change the existing Libint with ConfigureMake to CMakeMake, and I am not sure about the consequences. As I added a comment in the psi easyblock, it is possible to use the non-CMake build versions, but then one have to provide a corresponding Find<library-name>.cmake scipts. I think this would lead too far, so we do need the -PSI4 suffixes.
The basic problem is that the pirmary PSI4 distribution is with conda, so we do have to be happy that we can still build it from source.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

(especially, because configure possibilities are different in libxc with CMake and configure, not sure whats the situation in case of Libint)
I do feel that in this case the lets use all deps built by EB is just too much effort, because as a matter of fact, all extra suites should be EB build, like dkh, gdma, resp, snsmp2, etc. along with Libint, libxc, gau2grid, and pybind11) Well, this is a conswquence of a modularly built application....

Copy link
Member

Choose a reason for hiding this comment

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

Ugh, what a mess... OK, let's leave it as is then, and revisit later when we find time for it (yeah, right...)

@hajgato
Copy link
Collaborator Author

hajgato commented Nov 9, 2018

@boegel Changed CMake build type from None to EasyBuildRelease

@boegel
Copy link
Member

boegel commented Nov 14, 2018

Test report by @boegel
SUCCESS
Build succeeded for 5 out of 5 (5 easyconfigs in this PR)
node3136.skitty.os - Linux centos linux 7.5.1804, Intel(R) Xeon(R) Gold 6140 CPU @ 2.30GHz, Python 2.7.5
See https://gist.github.com/2a0ace91f874bb1f571662f8067090d7 for a full test report.

@boegel
Copy link
Member

boegel commented Nov 14, 2018

Test report by @boegel
SUCCESS
Build succeeded for 5 out of 5 (5 easyconfigs in this PR)
node2010.delcatty.os - Linux centos linux 7.5.1804, Intel(R) Xeon(R) CPU E5-2670 0 @ 2.60GHz, Python 2.7.5
See https://gist.github.com/ac7a6bbebfb27366a8adb160167495ea for a full test report.

@boegel
Copy link
Member

boegel commented Nov 14, 2018

Test report by @boegel
SUCCESS
Build succeeded for 5 out of 5 (5 easyconfigs in this PR)
node2676.swalot.os - Linux centos linux 7.5.1804, Intel(R) Xeon(R) CPU E5-2660 v3 @ 2.60GHz, Python 2.7.5
See https://gist.github.com/91bc79e4f4accd31b3dbcce710853a66 for a full test report.

@boegel boegel added this to the 3.8.0 milestone Nov 14, 2018
@boegel
Copy link
Member

boegel commented Nov 14, 2018

Going in, thanks @hajgato!

@boegel boegel merged commit 7c7d051 into easybuilders:develop Nov 14, 2018
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.

2 participants