{"action":"created","issue":{"url":"https://api.github.com/repos/psi4/psi4/issues/442","repository_url":"https://api.github.com/repos/psi4/psi4","labels_url":"https://api.github.com/repos/psi4/psi4/issues/442/labels{/name}","comments_url":"https://api.github.com/repos/psi4/psi4/issues/442/comments","events_url":"https://api.github.com/repos/psi4/psi4/issues/442/events","html_url":"https://github.com/psi4/psi4/pull/442","id":164355996,"number":442,"title":"CMake rewrite","user":{"login":"ryanmrichard","id":5280139,"avatar_url":"https://avatars.githubusercontent.com/u/5280139?v=3","gravatar_id":"","url":"https://api.github.com/users/ryanmrichard","html_url":"https://github.com/ryanmrichard","followers_url":"https://api.github.com/users/ryanmrichard/followers","following_url":"https://api.github.com/users/ryanmrichard/following{/other_user}","gists_url":"https://api.github.com/users/ryanmrichard/gists{/gist_id}","starred_url":"https://api.github.com/users/ryanmrichard/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ryanmrichard/subscriptions","organizations_url":"https://api.github.com/users/ryanmrichard/orgs","repos_url":"https://api.github.com/users/ryanmrichard/repos","events_url":"https://api.github.com/users/ryanmrichard/events{/privacy}","received_events_url":"https://api.github.com/users/ryanmrichard/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":70,"created_at":"2016-07-07T16:46:35Z","updated_at":"2016-08-05T13:48:44Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/psi4/psi4/pulls/442","html_url":"https://github.com/psi4/psi4/pull/442","diff_url":"https://github.com/psi4/psi4/pull/442.diff","patch_url":"https://github.com/psi4/psi4/pull/442.patch"},"body":"# Overview\r\nThis is version 2.0.0 of the Psi4 build system.  It is designed to speed-up build times as well as aid in transitioning to a more modular model driven by QCDB or incorporated into the Pulsar framework.\r\n\r\n## Disclaimer\r\nThis is *NOT* quite ready to merge yet, but represents a substantial change that I wanted developers to start looking at and provide feedback on.  In particular:\r\n\r\n- [x] Get remote builds via Travis-CI working\r\n- [x] @lothian 's coupled cluster code tests are failing (really could use help there)\r\n- [x] Fix sow/reap tests (not using installed data dir?)\r\n- [x] Failing test cases:\r\n      - [x] \t cbs-xtpl-freq\r\n      - [x]\t  fnocc2\r\n      - [x]  tu4-h2o-freq \r\n- [x] Hook back-up:  \r\n       - [x] chemps2\r\n       - [x] erd\r\n       - [x] dkh\r\n       - [x] gdma\r\n       - [x] ambit\r\n       - [x] pcmsolver\r\n- [ ] Get binary working (need help from @loriab)\r\n- [x] Test (and then most likely fix) plugin system\r\n- [x] Move GTFock to external\r\n- [x] Libint is now external, closes #355 \r\n\r\n## Speed\r\nBy means of comparison, on my box the old build system took 34 minutes and this build took 14 minutes.  Both build times are for a debug build, ignore the time to build Boost, use the default angular momentum, and are run on 6 cores.  Furthermore, this new build does a dramatically better job of tracking dependencies among files.  For example libint/libderiv should only build once and editing a single file in mints no longer triggers a full recompilation (of course touching a file like Molecule.h basically will).  This means subsequent builds should be dramatically faster, in my experience so far typically only the file that was changed needs recompiled.\r\n\r\n## Modularity\r\nUnder the old build the entire Psi4 build project had to track dependencies for all plugins and external interfaces.  This was quickly becoming a nightmare as evidenced by the 3,000,000 emails we have been getting about chemps2 breaking.  I have transitioned us to what is known in CMake land as a superbuild.  In such a build all pieces of a package, including the dependencies, optional modules, and the main project itself are treated as external projects.  Now each of these external projects simply worries about their dependencies and their building process.  Finally, because each external project is a good CMake project it makes a `XXXConf.cmake` file or a `FindXXX.cmake` file is included in `psi4/cmake`, which set `XXX_LIBRARIES` and `XXX_INCLUDE_DIRS` appropriately.  Hence all other external projects simply need to keep appending dependencies and everyone gets to play  nicely together.\r\n\r\n#  Detailed Change Log\r\n## Build scripts\r\nI metaphorically ripped all the old build scripts out and started fresh.  In my opinion the scripts are far easier to read.  It also means I may have not added some functionality back.  In particular there were many complicated scripts for setting compile flags.  At least on my box these scripts did not find all the optimizations to begin with (didn't turn on many of the Intel optimizations).  They also didn't respect some of the CMake variables.  The scripts themselves are designed to be quite terse by factoring out redundant code, although admittedly the `ExternalProject_Add` commands are still repetitive.  Also note I heavily use empty targets and variables to avoid lots of if/else statements.\r\n\r\n## Added an External Folder\r\nThe top-level Psi4 directory is primarily split into a directory, `psi4`, where the source for the core of Psi4 lives and a folder `external` where optional modules and dependencies live.  The general strategy for determining where something goes is if you personally didn't write the code, or it is intended to be used with many other packages, it probably falls under the category of going in the external folder.  Each package in `external` should be designed in such a manner so that it doesn't leak dependencies into the core.  For example if the package returns its results in a class `MyGreatMatrix` the wrapper code that copies it to a Psi4 `Matrix` should live in external.  This prevents Psi4 proper from having to know about `MyGreatMatrix`.  \r\n\r\n## Libraries that are not part of Psi4 core are now in external\r\nAlthough allowed by the more permissive licenses, it is somewhat dubious  to mix external libraries in with ones own source code.  Separating them makes it clear to others that these are not our codes, but used with (implicit) permission.  It also keeps the core free of the external dependencies in a far more controlled manner.  Right now the external libraries are: ambit, boost, erd, libint, efp, dkh, and gdma.  I still need to move GTFock there.\r\n\r\n## Headers\r\nI strongly encourage the Psi4 community to review what a forward declaration is as well as how to properly use header files.  I have posted a primer on our [BestPractices](https://github.com/psi4/psi4/wiki/BestPractices) wiki.  There is a lot of copy/paste going on in the preamble of source files.  You really should be assembling you includes/typedefs/etc. on a file by file basis to ensure you know what you are including, where it is coming from, and most importantly are only including what you need.  The reason the build is so much faster is that I have trimmed many of the unneeded includes.\r\n\r\n### Unified Header Inclusions\r\nAll include files are (or should be) relative to root (In practice I missed some, but they are in the same directory as the file they are included in and are fine for now).  The reason for this is we want to transition to a modular nature and it is easiest for the users of these modules if they only have to specify the Psi4 include directory as an include and not each subdirectory of it.  Note that this is how nearly every professional C++ library I am aware of does this (Boost, Eigen, TBB, ...).  The second (and arguably better reason) is doing it any other way makes the superbuild a lot harder when something is staged.\r\n\r\n## No More Convenience Headers\r\nConvenience headers, like `mints.h` are evil.  If they exist (like in many professional libraries) they are to be used by your users only.  The primary reason for avoiding them is that they increase compile times.  This is one of the primary reasons you had to recompile half of Psi4 when changing most files.  Lesser reasons:\r\n      - Harder to know exactly what you brought in (people duplicated the contents of `mints.h` all over the place). \r\n      - Can destroy distinction between interface/implementation classes if done incorrectly\r\n     - Splitting the header is a nightmare (as I can attest to) and is required if code gets factored, which really should happen to mints.\r\n\r\n## Removed psiconfig.h\r\nThe macros in it are easily replace by CMake's `add_definitions` function and doing so clears up the macro namespace considerably.  Furthermore a lot of those macros weren't even being used.\r\n\r\n## Static vs. Shared Psi4 Core libraries\r\nCMake provides the variable `BUILD_SHARED_LIBS` to toggle this.  We weren't respecting this and have been forcing static builds.  Again moving to modules is facilitated by shared libraries.  I will note that apparently libiwl can't be compiled dynamically.     \r\n\r\n## No More setup.py \r\n`setup.py` just wrapped the CMake call and rewriting it is an exercise in code duplication.  My understanding is the point of this file was meant to ease the transition to CMake from GNU Make.  Well, I think it's safe to say we've transitioned.\r\n\r\n## Angle Brackets for Includes Clean Up\r\nThe vast majority of includes that incorrectly used angled-brackets have been changed to quotes.  Although not exactly correct, for all intents and purposes, use quotes to indicate that a header file is part of the current project and angled-brackets indicate it is an external dependency.  This is not just semantics, it changes how the compiler behaves.  It also can lead to subtle hard to find bugs when you have very commonly named header files and you use directives like `using namespace std` all over the place.\r\n\r\n## Accessing Python Bindings\r\nIt used to be there were three ways to compile Psi4: for the conda binary, for an executable, and as a shared library that can be imported from Python.  There was a ton of redundancy between the latter two and I consolidated them into one.  Now all builds that are not making the conda binary, also produce callable Python bindings and an executable.  This is simply done by packaging Psi4 up into a giant .so that we then link psi4.cc against.  Because I am liable to forget here is an example of using Psi4 from python now, assuming `${CMAKE_INSTALL_PREFIX}/psi4/lib/psi4.so` is in your `PYTHONPATH`:\r\n\r\n~~~{.py}\r\nfrom psi4 import *\r\ninitialize()\r\n#This has to come after initialize or else the global variables aren't right\r\nimport driver\r\nset_global_option('BASIS', 'CC-PVDZ')\r\nxyzstring = \"\"\"\r\n    units angstrom\r\n    O     0.0000000000    0.0000000000   -0.0711762954\r\n    H     0.0000000000   -0.8916195680    0.5648097613\r\n    H     0.0000000000    0.8916195680    0.5648097613\r\n\"\"\"\r\nefp_init()\r\nmy_mol = Molecule.create_molecule_from_string(xyzstring)\r\nmy_mol.update_geometry()\r\ndriver.energy('scf',molecule=my_mol)\r\n~~~\r\n\r\nIt would be nice to consolidate this somewhat, but I'll leave that for later.  The segfault discussed on the forums is still present with this.\r\n\r\n## Fortran Name Mangling\r\nThis is a perpetual problem.   The previous CMake build relied on the Fortran/C interface capabilities of CMake to generate a header.  This seemed to work well, but as I understand it, it assumes  that the Fortran compiler CMake is currently using is the one that compiled all Fortran libraries.  Consequentially this does *NOT* necessarily provide you with the correct mangling for externally compiled Fortran libraries.\r\n\r\nFor Fortran libraries that we may are building, like ERD, DKH, etc. the name mangling should go through the CMake mangling detection mechanisms and the resulting header file included with the library.  This allows other projects to link to these libraries without having to know the mangling scheme.\r\n\r\n\r\n##BLAS and LAPACK\r\nSomewhat related to the previous point,  Psi4 proper needs to know the mangling for LAPACK and BLAS.  All distributions of BLAS have a header `cblas.h` (or `mkl-cblas.h` if you are using MKL), which does this for you.  Similarly, all versions of LAPACK provide `lapacke.h` (again MKL makes things difficult and calls the header `mkl-lapacke.h`).  Hence we really should be using these headers and not the stuff in `libqt` (or we should rewrite `libqt` in terms of those header files).\r\n\r\nOne of the other gotchas of BLAS/LAPACK is detecting the actual libraries.  The names are non-standardized and the CMake find_package for them is archaic.  For the moment I am using the scripts the old build used to find BLAS/LAPACK.  They are somewhat touchy, and overly verbose for my liking, but better than me having to write my own.  To my knowledge they don't worry about the Mac Accelerate framework so this is again an issue.\r\n\r\n\r\n## Minor Points Not Worth a Paragraph\r\n- exception.h has been moved into libpsi4util to be with its source file\r\n- process.h   has been moved into libparallel to be with its source file\r\n- `src/bin/psi4` is now `src/bin/psi4_main` to avoid name conflicts\r\n- `src/bin/efp` is now `src/bin/efp_interface` to avoid name conflicts\r\n- Most ConfigX files are gone.  From what I can tell, these files are what should be in the CMakeLists.txt wrapping project X.  That's where they now reside. \r\n- Some options are now gone\r\n  - ENABLE_MPI Psi4 proper contains no MPI now, if external projects want to include it that's fine\r\n  - ENABLE_OMP I never understood this one, just make it not required\r\n  - ~~ENABLE_ACCELERATE FindLAPACK knows about accelerate, if you want to use it ensure it is in your `CMAKE_PREFIX_PATH` variable~~\r\n- Accomplishes #355 "},"comment":{"url":"https://api.github.com/repos/psi4/psi4/issues/comments/237855000","html_url":"https://github.com/psi4/psi4/pull/442#issuecomment-237855000","issue_url":"https://api.github.com/repos/psi4/psi4/issues/442","id":237855000,"user":{"login":"ryanmrichard","id":5280139,"avatar_url":"https://avatars.githubusercontent.com/u/5280139?v=3","gravatar_id":"","url":"https://api.github.com/users/ryanmrichard","html_url":"https://github.com/ryanmrichard","followers_url":"https://api.github.com/users/ryanmrichard/followers","following_url":"https://api.github.com/users/ryanmrichard/following{/other_user}","gists_url":"https://api.github.com/users/ryanmrichard/gists{/gist_id}","starred_url":"https://api.github.com/users/ryanmrichard/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ryanmrichard/subscriptions","organizations_url":"https://api.github.com/users/ryanmrichard/orgs","repos_url":"https://api.github.com/users/ryanmrichard/repos","events_url":"https://api.github.com/users/ryanmrichard/events{/privacy}","received_events_url":"https://api.github.com/users/ryanmrichard/received_events","type":"User","site_admin":false},"created_at":"2016-08-05T13:48:43Z","updated_at":"2016-08-05T13:48:43Z","body":"@bennybp I'm trying to switch the `psi4_add_library` macro from imported to public as we speak.  The error I was getting was something about the alias target `PCMSolver::PCMSolver`, which I appear to have fixed; was this what you were also getting?"}}
{"actor":{"display_login":"ryanmrichard"}}