new cmake harness, round 3
~At present, this PR is so EFV can oversee proposed changes to #205. It configures, but I haven't tried so much as building the compiler.~
Added 19 Jan
- [x] switched boost target from generic
Boost::boostto h-oBoost::headersallowed in CMake 3.15 - [x] removed some cmake min version boilerplate left over from external_project structure
- [x] forced
add_library(libint-libcompiler STATICafter build failed withBUILD_SHARED_LIBS=ON - [x] updated
find_project(Pythonand result vars since CMake Python detection improved much around 3.15 - [x] change DATADIR so that it's relative path in Config
$<INSTALL_INTERFACE:DATADIR="\$\{_IMPORT_PREFIX\}/${CMAKE_INSTALL_DATADIR}"> - [x] rearrange the six exported libraries into defines, features, properties, includes, link_libraries
- [x] switches
${PN}and${NS}abbreviation variables to${L2}as the former generic names can get overwritten by a call (e.g.,find_package()where another project assigns them) - [x] return to 3-char orderings in Config (e.g.,
gssfor gaussian-standard-standard) since I think the 2-charsswere in anticipation of runtime-selectable solid harmonics. (less a priority for psi now)
WIP c. 19 Jan
- ~bumped CMake to 3.18 for list sorting in
int_am.cmake. will revert to 3.16 at the end unless new requirements surface.~ - ~start upgrade guide and options description in
INSTALL.md~ - ~detect Eigen if needed in Config file~
- ~FindTargetEigen3~
- ~int_am finalization (need to show where #205 not behaving. need to handle G12 properly)~
- ~organize exports to targets files better~
- ~handle library targets as components in Config better~
Added 20 Jan
- [x] moved configure.ac and config.h.in to a
archived_libool/dir so they can't participate unbeknownst to us - [x] have only the 3 definitive version lines (version, buildid, soversion) in top-level CM file. leave derived version vars and handoff to library target to cmake modules. define versions in
project()(top-level and library export) so that usual_MAJOR, PROJECT_TWEAK, etc. that ppl may expect are defined - [x] install basis/ into versioned location again. should it really be
share/libint/version/basis, notlibint2?
Added 21 Jan
- [x] check user can overwrite all the install locations and they get passed from generator correctly and all captured in the final library install to prefix.
- [x] gave up defaulting CMAKE_INSTALL_LIBDIR to lib. probably more natural to stick with standard GNUInstallDirs. renamed (back) the custom install vars LIBINT2_INSTALL_BASISDIR and LIBINT2_INSTALL_CMAKEDIR so as to not invent new vars starting CMAKE_
- [x] address pkgconfig paths issue mentioned at 205
- [x] expand
Libint2::aliases so that available internally. use these for tests and drop theint-library, etc. series.
Added 24 Jan
- [x] simplify enable_testing and build_testing sequences for new cmake and test them.
- [x] add onebody, header-only, and pure_am components for psi's benefit. document/copy-over program-specific parameters.
- [x] make -> cmake for doxygen. it generates files, but I haven't examined them.
- [x] take adv of newer cmake to skip DESTINATION where can. Make sure each install() for library goes to
Libint2_RuntimeorLibint2_Development
Added 28 Jan
- [x] rename
FindMPFR.cmaketoFindMultiprecision.cmaketo handle gmp, gmpxx, and mpfr targets. Based on offical cmake module FindGSL.cmake to hopefully handle Windows, too. Seek correct targets in build_libint and library. - [x] remove old-style cmake variables like
Libint2_LIBRARIESfrom config. added backLibint2_EXT_VERSION. Rearrange config so no targets created before "found" assured. - [x] make sure CMAKE_PREFIX_PATH passes to ExternalProject even when list by placing in Cache. Only install bundled Boost if CXX activated
- [x] try to standardize cmake formatting (on lines I've touched for worthier reason) to 4 spaces normal and 2 spaces w/i command.
- [x] Fortran work. Setting both
CMAKE_Fortran_MODULE_DIRECTORYand propertyFortran_MODULE_DIRECTORYseemed redundant. Switched to only the latter and added the path to include directories on the target. Looking on the internet, where to install module files looks like it has no guidelines (esp. since compiler-version-dependent) so added an install knob. Added fpic when necessary so tests work with shared lib.
Added 12 Feb
- [x] renamed GHA workflow to "CI". split it into two jobs, the first of which
build_repo, as before, builds and runs the compiler, builds the library, tests the library, tests a library consumer for Lin/Mac Debug/Release, all static libs. To this was added artifacting the built tarball and building on Windows (Release only). The Windows build is an expected fail -- the generated source is faulty. The second and new jobbuild_exporttakes the linux tarball artifact and builds it on Lin/Mac/Win, shared libs where possible, with some additional dependency quirks testing. Changed build matrix from include format to cfg format, as I think it's easier to read. Switched Debug/Release EP/FetchContent pairs as Windows only works with 1/4 combinations. - [x] Reverted minimal CMake to 3.16 with a fn in int_am.cmake . Test exactly 3.16 in GHA.
- [x] Bump CMake version to 2.7.1 from 2.7.0-beta.6 to match configure.ac
- [x] Added an option
REQUIRE_CXX_API_COMPILEDto turn on/off the compiled C++11 interface apart from the header-only library. It's on by default, so no change. - [x] Set up
ENABLE_T1G12_SUPPORTmissing before. - [x] Added G12 ints. new macro in int_am.cmake to handle non-list, etc. and components
- [x] Same for G12DKH but these throw error at compiler exe or lib build stage.
- [x] renamed
int-cxx-{static,shared}toint-cxx-compiled-{static,shared}for clarity - [x] reworked Eigen3 detection. copied a FindEigen3.cmake from latest Eigen release. This is installed with L2 and the local path is prepended so this FindEigen3 is always used. The FindEigen already handles (1) finding installations with a Eigen3Config.cmake, (2) seeking if the Eigen3_INCLUDE_DIR already in cache, (3) searching for header like a traditional FindPkg.cmake. It always returns a target, Eigen3::Eigen. This has been modified to address some of your concerns. (1) turned off build package registry to avoid the ephemeral build trees. (2) If an option
LIBINT_LOCAL_Eigen3_FINDis set, the FindEigen instead looks to import a local L2-defined target (see below) to ensure ABI consistency. This is off by default. The FindEigen3 detection is shared by the library build and libint2-config.cmake - [x] Eigen3 handling on the src/lib/libint/CM.export side is outsourced to FindEigen3. If
LIBINT_LOCAL_Eigen3_INSTALLis set, the detected eigen3 is installed as a target in a separate file. This is off by default, but I don't care if it's on by default, so long as it's turn-off-able and the _FIND option is off by default. - [x] fixed some locale problems for Windows in basis.h. Added some ifdefs for gnu compilers that I hit when using the wrong Windows compiler. Left them, but not really needed.
- [x] removed a seemingly redundant basisset file. no trouble so far.
- [x] Fixed Mac shared libraries that weren't linking
- [x] Half fixed Win shared libraries that aren't exposing fn symbols (fixed with windows_export_all_symbols) or libint2_build_* data symbols (not fixed; see issue)
- [x] half install target for fortran interface. (target installed, export written but not installed.) I don't know if it's wanted or generally redistributable enough.
- [x] consolidated the main install(target and install(export according to my latest reading of best cmake practices.
WIP
- [ ] Still working on INSTALL.md and general tidying
- [ ] Do you want
ENABLE_XHOSTto autoselect optimization for current instruction set? It's gotten more complicated (2 -> 20 lines) - [ ] Should I mark the name
int2-cxxas provisional? I find the name confusing, and the library isn't testable. - [ ] The components names C, CXX, CXX_ho, I keep getting wrong, which probably means they're not intuitive.
- [ ] Merge README.md and INSTALL.md?
@loriab CI passes!
how painful would be to revert to cmake 3.16?
@loriab CI passes!
Thanks!
How painful would be to revert to cmake 3.16?
It won't be too bad. I figured I'd keep it at 3.18 for int_am readability for now (2 lines to compute each max instead of ~7) then revert to 3.16 at the end. I have updated some Python and Boost things that could be used only >=3.15.
Sorry to leave CI broken after you so kindly fixed it. This shows a strategy for separating machine-path-dependent targets (that you described the need for but are unsuitable for distribution) from the usual redistributable targets. This also fixes the full paths in the installed target tree for the latter. I'm still working on shifting eigen3 detection, so CI has no chance at the moment.
@loriab no worries
BTW I am not sure I understand why we need to have the "nonlocal" (path-independent) targets. If you allow the user to discover Eigen3 on their own or provide their own Eigen3 target there is no way to make sure that it is configured correctly (e.g. the one used by Libint to pre-instantiate Engine used Eigen3 configured to need BLAS/LAPACK ... there is no way to test Eigen3's configuration without much try_compile code).
This is a common scenario for C++ and even for C code. E.g. CMake's standard MPI targets are ABI-specific, so it is not possible to say find_package(MPI COMPONENT mpich_abi ... ) or find_package(MPI COMPONENT sgi_abi ... ) ... the only way to avoid ABI problems in general is to hardwire to the particular instance of a target.
Is this a requirement motivated by package maintenance??
BTW I am not sure I understand why we need to have the "nonlocal" (path-independent) targets. If you allow the user to discover Eigen3 on their own or provide their own Eigen3 target there is no way to make sure that it is configured correctly (e.g. the one used by Libint to pre-instantiate Engine used Eigen3 configured to need BLAS/LAPACK ... there is no way to test Eigen3's configuration without much try_compile code).
This is a common scenario for C++ and even for C code. E.g. CMake's standard MPI targets are ABI-specific, so it is not possible to say find_package(MPI COMPONENT mpich_abi ... ) or find_package(MPI COMPONENT sgi_abi ... ) ... the only way to avoid ABI problems in general is to hardwire to the particular instance of a target.
Is this a requirement motivated by package maintenance??
Yes, I'm thinking of packaging and accommodating the up to three computers that build library source, build library, and build library consumer. All I really want in the end is:
- [x] be able to generate a libint2 library source tarball to build on Lin/Mac/Win
- [ ] agree amongst the packagers (fedora, debian, conda-forge) on a set of libint config options such that a single built library can service all open-source QC programs (built packages) and be checkable through components at library detection time.
- [ ] be able to continue to supply a built libint2 conda package so that psi4 devs and users can build psi4 against it pretty cheaply. Almost all use the conda eigen3 package, of fixed relative location wrt libint, but that's not a constraint.
I think I'm reading code right that the Libint2 cxx interface at present is header only. So the detection at library-build time in src/lib/libint/CMakeLists.txt.export is perfunctory so that the eigen target can be recorded in libint2-config.cmake as a dependency of Libint2::cxx interface library target. The eigen code only gets compiled within the libint repo for the tests that use the cxx interface. If I have all that right, I feel on pretty firm cmake footing that there shouldn't be full paths in the config file and one doesn't generally install external dependency targets (https://cmake.org/cmake/help/latest/guide/importing-exporting/index.html#creating-relocatable-packages). The latter is unusual enough that there's a new v3.21 capability to allow it (https://cmake.org/cmake/help/latest/command/install.html#installing-runtime-dependencies). I'd expect the eigen detection to happen in a find_dependency(eigen-like) in the libint-config file. I'm working on wrapping the call so it allows plain header location and avoiding the build tree, not just the discoverability of Eigen3Config.cmake . In this libint2-cxx-interface-header-only case, this eigen code detected at built-libint detection time is the only eigen code involved, so no need to match up exact installations.
Your plan of non-header-only Libint2 cxx interface surprised me. I gather that the int-cxx-shared target becomes a tangible library with compiled code, not just an interface library. I accept that the same eigen code and offloading defines ought to be used for both libint-building and libint-consumer-building. This is fairly easy when packagers control both the libint2-building and the e.g., psi4-building steps because they'll use common eigen conditions. I'm not sure how it should work for the more general cmake case, though. Just vendorizing the eigen code into include/libint2/eigen/ doesn't work because it's not just the eigen code you want shared between libint2 and the qc program but also the offloading defines and attendant library linking. Since you seemed to assume that libint2 and QC consumer were build on the same computer anyways by the full paths to eigen, I figured a workaround to preserve normal cmake relocatability for the header-only cxx interface and accommodate the common-computer non-header-only cxx interface you describe would be to separate the targets so packagers can decline to install the latter while users can request the latter.
I hope I'm understanding your points and plans better in these posts. Mostly I've been focusing on the existing header-only state. Thanks for you patience. Pinging @susilehtola in case there's a packaging aspect/constraint that I'm missing, being mostly conda-focused.
@loriab due to the header-only inefficiencies the plan to make C++ API a proper (compiled) library has been around for awhile (for as long as engine.impl.h existed). There will be no impact on current API users (like Psi), and switching to the compiled version will be trivial: cxx to be a header-only C++API and cxx_{shared,static} a compiled version (for now only including Engine, eventually might include more).
I agree that for careful users the ABI issues will not appear. But I'm just giving you a heads-up for what's likely to happen (and has happened to me multiple times before ... so experts are not immune). Since ABI mismatches are impossible to detect at build-time and in general extremely painful to diagnose the ABI mismatches in my book are more evil than hardwiring to the particular target instance ... perhaps the hardwiring should be the default with an opt-out for experts? OR the other way around? (the latter makes less sense to me: the only reason to hardwire search paths is for an average user, not an expert).
P.S. Eigen is just an fn pain. I'd love to be able to just restrict to config-equipped Eigen but the amount of pushback I get along the lines of "but Eigen is a header-only library!!!! I should not need to install it" is tiresome.
@loriab btw I made the int-cxx-{static,shared} a compiled library. The header-only (interface) version is int-cxx-headeronly-{static,shared}. Should probably not be used by anyone since cmake is now the only way the C++ interface can be used by mortals
@loriab btw I made the int-cxx-{static,shared} a compiled library. The header-only (interface) version is int-cxx-headeronly-{static,shared}.
Good, thanks, it'll be nice to look at something concrete.
If both int-cxx-headeronly-{static,shared} and int-cxx-{static,shared} targets are available for downstream to choose, that'll work well for me. Psi (for now) will probably stick with the headeronly since that can be distributed safely, eigen-wise. (Thanks for the heads-up on the ways eigen can go wrong when spread across two build-times.) And libint consumers building libint and downstream on the same machine can take advantage of the compiled int-cxx-{static,shared} targets where the full path to local eigen is exported.
Should probably not be used by anyone since cmake is now the only way the C++ interface can be used by mortals
... unless this sentence refers to the header-only target, in which case I'm not following.
I'll get your #205 changes rebased through and work from there.
Fedora wise I am not aware of any issues other than the issues of incompatible orderings, normalizations etc. Any dependencies like Eigen3 will be pulled from the Fedora repositories, so everything is built in a consistent manner. MPI versions are compiled separately for MPICH and OpenMPI, although I'm not sure why Libint would need that? ABI incompatibilities (changing compile time options) are handled by a ABI version tag in the RPM spec file, so if the options need to be changed then all dependent packages are rebuilt to satisfy the broken dependencies.
@evaleev, this PR isn't ready for review yet, but if you have a chance, I'd appreciate if you could look over this docs table https://github.com/loriab/libint/blob/new-cmake-harness-lab-rb1/INSTALL.md#prerequisites and footnotes for correctness. I'm sorry that it's a little hard to edit -- GH web interface is probably easiest or just let me know the corrections. Thanks!
@evaleev, I think this is ready for review. I'm still working on the INSTALL.md docs, but I'm satisfied with the technical situation. If you approve of the general structure and solutions, I'd like to advise packagers that it's ready to test soon, as the problems they find are probably in my court rather than yours. Let me know if you'd like to Zoom to talk over reasoning and concerns in real time for efficiency.