-
Notifications
You must be signed in to change notification settings - Fork 440
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
add optional ecpint to buildsys #2368
Conversation
Is this PR just one fragment of what will be needed for Psi to use |
Seems to need the MSVC cmath fix https://github.com/psi4/psi4/blob/master/psi4/CMakeLists.txt#L224 |
Thanks, @hokru, I've verified that the Windows build works. Then turned off building ecpint by default since it shouldn't have been in the first place. Right, @JonathonMisiewicz, this is just the buildsys piece. This'll help Andy's #2135 |
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.
A couple minor clarifications that we better address now rather than risking forgetting about them.
@@ -63,7 +63,7 @@ list(APPEND CMAKE_MODULE_PATH ${CMAKE_CURRENT_SOURCE_DIR}/cmake) | |||
# - Python_INCLUDE_DIR "Path to the python include files (e.g., /path/to/include/python3.7)" | |||
# - SPHINX_ROOT "Root directory for Sphinx: 'bin/sphinx-build' (or similar) should be in this dir." | |||
# | |||
# For any ${AddOn} of: ambit, CheMPS2, dkh, libefp, erd, gau2grid, gdma, Libint2, PCMSolver, pybind11, pylibefp, | |||
# For any ${AddOn} of: ambit, CheMPS2, dkh, ecpint, libefp, erd, gau2grid, gdma, Libint2, PCMSolver, pybind11, pylibefp, |
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.
Confused about the nomenclature here. ecpint
as opposed to libecpint
, but then there's libefp
, libint2
...
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.
yeah, it's largely determined by what ppl define for project(my_Name)
in cmake. after that, cmake is highly patterned on that word (including the capitalization). The libecpint project isn't entirely consistent on this, but if I can get away with "ecpint" (and I seem to be able to so far), I think that's preferable. other opinions?
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.
I prefer libecpint
purely because that's the name of the repo. I'm not sure @robashaw has an opinion?
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.
he's using ecpint
for the find_package
name and the namespace
https://github.com/robashaw/libecpint/blob/master/src/CMakeLists.txt#L81-L92 so sticking with that for now. can still be rejiggered after merge if Shaw wants to make a ruling.
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.
This is my fault for making a dumb decision early on (back when I didn't understand CMake at all), but yes I think it's safest keeping everything as "ecpint" to be consistent. Also I've seen the PR on libecpint and I'll hopefully get a chance to check and merge it soon.
Description
Basic can-get-it-in-psi for Rob Shaw's https://github.com/robashaw/libecpint .
Todos
Questions
Checklist
Status