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

all-stage CMake #148

Closed
wants to merge 55 commits into from
Closed

all-stage CMake #148

wants to merge 55 commits into from

Conversation

loriab
Copy link
Collaborator

@loriab loriab commented Sep 27, 2019

WIP

This is the start to CMake as a replacement for libtool builds. So far it builds the generator, runs it, and builds the library. That may sound like good progress, but only one set of input options examined.

closes #147

@loriab
Copy link
Collaborator Author

loriab commented Sep 27, 2019

  • (from other thread) There's not a naming standard that I know of, but find_package(Libint2), find_package(Libint3), and find_package(Libint) should do nicely for 2, 3, 1, respectively. If I had been more thoughtful, I'd have written the last as Libint1, but there's no point in switching now.

  • What does __COMPILING_LIBINT2 indicate? Or, is there any phase (building generator, building library, building export, building exported library) in which it should not be set?

  • Do I gather correctly that the Cartesian shell ordering is used in building the generator, while the spherical shell ordering is not, thus the generated code has fixed Cart ordering but flexible (until library configuration time) spherical ordering? When does shell_set=standard/orca get fixed?

@loriab
Copy link
Collaborator Author

loriab commented Nov 26, 2019

As an update,

  • there's a Libint2 conda package for linux compiled according to https://github.com/psi4/psi4meta/blob/master/conda-recipes/libint2/build.sh#L38-L62 available via conda install libint2 -c psi4/label/dev
  • my eventual plan for this PR is to have a superbuild with 3 serial steps.
    1. generator: download repo source, build build_libint executable,
    2. package: download step is running build_libint and build step is make the tarball,
    3. library: download step is unpack the tarball, build step is build the library.
  • current state is cmake is a 2 step akin to the libint1 cmake, where the second step is download by running build_libint and build the library directly.
  • I'd say the replacement of configure by CMakeLists.txt is in pretty good shape at present. Though I've been focussing on the c++11 interface, not the earlier (and perhaps more widely used) one.
  • would it be ok to initialize Azure DevOpts on this repository to start checking a Windows build? Azure is more powerful and generous than Travis but harder to use. Psi just moved our Linux builds over from travis to azure, but it's fine to have azure for Win and travis for Lin here.

@loriab
Copy link
Collaborator Author

loriab commented Dec 10, 2019

While Psi4 is still in the testing and tuning phases of Libint2 integration (psi4/psi4#1721), and this PR isn't complete (Windows not fully working), most major features are in, and it's ready for consideration. @evaleev, please let me know if you have any questions, especially about the strategy.

Major features

  • configure translated to CMake
  • three-step build of generator/export/library. export tarball built always and incorporated into library build if proceeding beyond export step
  • eventually, all the configure, Makefiles, and export directory (except a couple files to relocate) can be deprecated.
  • ported to Windows
  • resulting built library has .pc file for the libtool folks. Also has Libint2Config.cmake that supports components like sss to indicate all standard orderings and g4 to indicate that ERIs of at least deriv=1 and am=4 have been built so that packages can determine if a Libint is right for them at detection time.
  • testing is active, though I think it could be simplified.

Not fixed

  • Windows version builds and links to psi4 and emits at least some correct integrals, but not all correct.
  • Should enable Azure on libint repo
  • Needs to play nicer with existing travis testing

Won't fix in this PR

  • support for non-C++11 API library (int2 as opposed to cxx target)
  • support for Fortran interface
  • support for pre-C++11 compilers
  • there's some options still #undef in the config.h.cmake.in file. I'm adding them as I figure out what they do
  • other details psi4 hasn't needed -- boost tarball, basis set library, non-mpfr

@loriab
Copy link
Collaborator Author

loriab commented Sep 11, 2020

This is rebased and passing.

@loriab
Copy link
Collaborator Author

loriab commented Jan 5, 2021

Update from above. @evaleev, what's your hoped/needed timeframe for this PR? Things that must be done, imo, is eigen/mpfr dep detection. Thing that ought to be done is some manner of CI testing the pure cmake build. Otherwise, this is working as well as Psi4 needs it to.

Major features

  • configure translated to CMake
  • three-step build of generator/export/library. export tarball built always and incorporated into library build if proceeding beyond export step
  • eventually, all the configure, Makefiles, and export directory (except a couple files to relocate) can be deprecated.
  • ported to Windows
  • resulting built library has .pc file for the libtool folks. Also has Libint2Config.cmake that supports components like sss to indicate all standard orderings and g4 to indicate that ERIs of at least deriv=1 and am=4 have been built so that packages can determine if a Libint is right for them at detection time.
  • testing is active, though I think it could be simplified.

Not fixed

  • FIXED Windows version builds and links to psi4 and produces a clean test suite.
  • QUERY Should enable Azure on libint repo. Or do you want GitHub Actions? Is Travis still working here?
  • Needs to play nicer with existing travis testing

Won't fix in this PR

  • support for non-C++11 API library (int2 as opposed to cxx target)
  • support for Fortran interface
  • support for pre-C++11 compilers
  • there's some options still #undef in the config.h.cmake.in file. I'm adding them as I figure out what they do
  • EDITED & QUERIES other details psi4 hasn't needed -- boost tarball, basis set library, non-mpfr
    • DONE boost tarball distribution in source tarball and handling of boost in Libint2Config.cmake.
    • QUERY eigen headers not done. Being header-only, should/can these be tarballed up, too, like boost?
    • BLOCKER I have a new FindMPFR.cmake for the mpfr/gmp detection, but it was defying me. I haven't detangled and tested the transitive header/lib deps here. Most Psi4 users can ignore this problem b/c they build with conda, but this is a barrier to generic Linux compile.
    • NEW Portable libint #190 solid-harmonic order as runtime option rather than build-time. Beyond this PR, but it will require a slight revamp of the cmake component interface when ready.

@loriab loriab mentioned this pull request Jan 5, 2021
@evaleev
Copy link
Owner

evaleev commented Jan 26, 2021

@loriab I am starting on this now. Looks like the biggest missing piece is docs update ... what are you thinking re: Eigen + MPFR? FetchContent, if missing?

@loriab
Copy link
Collaborator Author

loriab commented Jan 26, 2021

Great, thank you!

On docs, I was supposing the in-file descriptions https://github.com/evaleev/libint/pull/148/files#diff-1e7de1ae2d059d21e1dd75d5812d5a34b0222cef273b7c3a2af62eb747f9d20aR16-R195 would do and be most likely to be kept up-to-date. Were you thinking about an alternate to https://github.com/evaleev/libint/wiki#compiling-libint-compiler? For the per-qc-program options section, some psi4 guidance is here (https://github.com/psi4/psi4/blob/master/external/upstream/libint2/CMakeLists.txt). And an example of the production build and absolute smallest possible build (for static analysis) are:

cmake -H. -Bbuild50 \
    -GNinja \
    -DCMAKE_INSTALL_PREFIX=/home/psilocaluser/gits/libint2/install-50 \
    -DCMAKE_CXX_COMPILER=$CXX \
    -DCMAKE_C_COMPILER=$CC \
    -DEigen3_DIR=$CONDA_PREFIX/share/eigen3/cmake/ \
    -DMPFR_ROOT=$CONDA_PREFIX \
    -DBOOST_ROOT=$CONDA_PREFIX \
    -DLIBINT2_SHGAUSS_ORDERING=gaussian \
    -DERI3_PURE_SH=OFF \
    -DERI2_PURE_SH=OFF \
    -DBUILD_SHARED=ON \
    -DBUILD_STATIC=OFF \
    -DBUILD_TESTING=ON \
    -DENABLE_ERI=2 \
    -DENABLE_ERI3=2 \
    -DENABLE_ERI2=2 \
    -DENABLE_ONEBODY=1 \
    -DDISABLE_ONEBODY_PROPERTY_DERIVS=ON \
    -DMULTIPOLE_MAX_ORDER=4 \
    -DWITH_ERI_MAX_AM="7;7;4" \
    -DWITH_ERI3_MAX_AM="7;7;5" \
    -DWITH_ERI2_MAX_AM="7;7;5" \
    -DWITH_MAX_AM="7;7;5"
   -DENABLE_ERI=0 \
    -DENABLE_ONEBODY=-1 \
    -DDISABLE_ONEBODY_PROPERTY_DERIVS=ON \
    -DMULTIPOLE_MAX_ORDER=0 \
    -DWITH_ERI_MAX_AM="1" \
    -DWITH_MAX_AM="1"

Were there some docs sections you'd like me to draft?

@loriab
Copy link
Collaborator Author

loriab commented Jan 26, 2021

I haven't usually built or offered to fetch-and-build software that's non-QC (pb11 excepted). So I was going to expect mpfr and eigen to be present and fail if not. With eigen being header-only, I can see treating it like boost and unpacking and installing and exporting a tarball. gmp/mpfr is trickiest, with an actual library. I've written a fancier FindMPFR.cmake, but its integration is WIP since I haven't gotten it to link libint2 with a system mpfr. What do you think of handling eigen like boost?

@evaleev
Copy link
Owner

evaleev commented Jan 26, 2021

@loriab

  • dox: at the minimum we need to update INSTALL (while at it might as well rename to INSTALL.md). End users do not seem to be OK with reading source, not sure why ;) Long-term I'd like to convert the wiki into markdown embedded into Doxygen, along the lines of what I did for TiledArray: https://valeevgroup.github.io/tiledarray/dox-master/

  • building exported lib: how about just FetchContenting the tarball as subproject? From our multiyear experience with ExternalProject_add it is tedious to maintain all cmake args to the external project's cmake (there is no transparent way to to pass all of them) and handle all corner cases. E.g. currently configuring library is broken when I try on arm64/macOS without any args because CMAKE_C_COMPILER is not specified:

-- The C compiler identification is unknown
CMake Error at tests/CMakeLists.txt:33 (enable_language):
  No CMAKE_C_COMPILER could be found.

  Tell CMake where to find the compiler by setting either the environment
  variable "CC" or the CMake cache entry CMAKE_C_COMPILER to the full path to
  the compiler, or to the compiler name if it is in the PATH.


-- Configuring incomplete, errors occurred!
  • probably want to eliminate the autotools harness.

  • re: dependencies ... in my experience need to use systemwide or user-provided packages, but fetchcontent if missing, for all dependencies. Libint is not bad itself, only has < 5 deps at the moment, but some parts of the code could use e.g. better transposes (e.g. if MKL if available) ... I would avoid header-only route as its difficult to maintain ... header-only = macros instead of proper build system (basically, consumer ends up duplicating cmake code that belongs in the "header-only" package)

  • some options seem to duplicate standard CMake vars ... ? should we use CMAKE_POSITION_INDEPENDENT_CODE (https://cmake.org/cmake/help/latest/variable/CMAKE_POSITION_INDEPENDENT_CODE.html) instead of BUILD_FPIC

Anyway, I'll keep poking and pushing as I see fit.

@evaleev
Copy link
Owner

evaleev commented Jan 26, 2021

@loriab FYI: I made a sister branch (#205), instead of overwriting your work, where I eliminated almost all (1 remains) ExternalProject calls. This should make it easier to make sure the cmake state is the same (incl imported targets) throughout the project. Currently library is generated successfully but I messed something up ... config2.h.cmake.in is not found when I am configuring the generated library.

@loriab
Copy link
Collaborator Author

loriab commented Jan 26, 2021

dox: at the minimum we need to update INSTALL (while at it might as well rename to INSTALL.md). End users do not seem to be OK with reading source, not sure why ;) Long-term I'd like to convert the wiki into markdown embedded into Doxygen, along the lines of what I did for TiledArray: https://valeevgroup.github.io/tiledarray/dox-master/

I'll work on a draft of INSTALL modifications

probably want to eliminate the autotools harness.

Do you mean the pkgconfig files? I've considered them harmless to generate for libxc, for example, just in case downstream wants to use autotools detection. Or do you mean configure.ac entirely? Fine by me, but this has had zero configuration and testing for C and Fortran interfaces.

some options seem to duplicate standard CMake vars ... ? should we use CMAKE_POSITION_INDEPENDENT_CODE (https://cmake.org/cmake/help/latest/variable/CMAKE_POSITION_INDEPENDENT_CODE.html) instead of BUILD_FPIC

Fine by me. BUILD_FPIC was an option I distributed widely in my early cmake days. It may have been useful then when building both shared (w/fpic) and static (w/o) before I trusted the cmake property.

Anyway, I'll keep poking and pushing as I see fit.

Sounds great, thank you! Let me know when you'd like a psi4 integration test performed. I can do Linux and Mac by hand, but right now the only way to check Windows (without imposing on @andysim) is to have psi4's Azure generate/export/build or build.

@evaleev
Copy link
Owner

evaleev commented Jan 27, 2021

I'll work on a draft of INSTALL modifications

OK, thanks! Can use the wiki installation guide augmented with a list of requirements CMake variables

Do you mean the pkgconfig files? I've considered them harmless to generate for libxc, for example, just in case downstream wants to use autotools detection. Or do you mean configure.ac entirely? Fine by me, but this has had zero configuration and testing for C and Fortran interfaces.

The latter: remove configure.ac entirely. There are too many issues, e.g. #204 , and I have not tries building exported libs using autotools in years.

Fine by me. BUILD_FPIC was an option I distributed widely in my early cmake days. It may have been useful then when building both shared (w/fpic) and static (w/o) before I trusted the cmake property.

I think we best replace

Sounds great, thank you! Let me know when you'd like a psi4 integration test performed. I can do Linux and Mac by hand, but right now the only way to check Windows (without imposing on @andysim) is to have psi4's Azure generate/export/build or build.

I'm almost done with the first round of revision ... lots of work remains still (Fortran, etc.) but getting there. Thanks for getting this started!

@evaleev evaleev mentioned this pull request Jan 27, 2021
16 tasks
@evaleev
Copy link
Owner

evaleev commented Jan 27, 2021

lives on as #205

@evaleev evaleev closed this Jan 27, 2021
loriab added a commit to loriab/libint that referenced this pull request Jan 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Libint2 cmake
3 participants