Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
{chem}[intel/2018b] PSI4 1.2.1 (REVIEW) #7109
Changes from 7 commits
6b40964
6385e41
6d865e5
2c3c9d3
78c2eb0
c9661df
5e6233b
04616b2
ae38b01
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
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)?There was a problem hiding this comment.
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
andLibint
, because those should be installed withCMake
, with specific options. The currentlibxc
andLibint
are built withconfigure
, not withCMake
, plusPSI4
have specific requirements forLibint
with a unifiedlibderiv
includedir, and with a different scheme oflibint-max-am
andlibderiv-max-am1
. Further compilations, that with theCMake
cannot enabler12
options forLibint
. As a consequence, I should use a specific version ofLibint
for PSI4, (which would be not compatible with a possible future version ofCP2K
and a new version oflibxc
which was built withCMake
instead of the existingconfigure
. The moment I faced this problem, thepsi
easyblock was already done, and I did not delete that part.If you wish, I can make the PSI4 specific
Libint
andlibxc
easyconfigs with the-PSI4
suffix, and/or I can delete theLibint
andlibxc
parts from thepsi
easyblock.There was a problem hiding this comment.
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
andLibint
as dependencies ourselves? Are they downloaded on the fly during the installation ofPSI4
? If so, that's a good reason to include them as dependencies instead imho, even if they are very specific toPSI4
(but we should try and make them a bit more generic if possible by not usingPSI
in theversionsuffix
).There was a problem hiding this comment.
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 withCP2K
(see for specific include dir). Maybe in the case ofLibint
we can make a more generic easyconfig, but then we have to change the existingLibint
withConfigureMake
toCMakeMake
, and I am not sure about the consequences. As I added a comment in thepsi
easyblock, it is possible to use the non-CMake build versions, but then one have to provide a correspondingFind<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.There was a problem hiding this comment.
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 ofLibint
)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 withLibint
,libxc
,gau2grid
, andpybind11
) Well, this is a conswquence of a modularly built application....There was a problem hiding this comment.
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...)