Modernisation of CMake scripting to simplify build of FullSimLight
With Athena now using Geant4 static/big libraries, optimization work has shifted to look at use of LTO/PGO. As these are relatively involved tasks, a first step is to use FullSimLight as a proof-of-principle for the flags and operations needed to support this. Though this work won't (at least at this stage) be integrated into GeoModel/FullSimLight, the need to hack at the build scripts has highlighted some modernizations/simplifications that can be made independently right now. These are not only useful to assist the LTO/PGO work, but for general efficiency and clarity of the build.
- Bump CMake minimum to 3.16, tested up to 3.26. Could be reduced a little, but 3.16 or higher is now pretty common across all platforms (see Repology listing).
- Allows removal of some workarounds needed with older versions
- If FullSimLight also gets used to test use of Celeritas/AdePT GPU simulation libraries for ATLAS, a more modern CMake will be needed for better CUDA support.
- Consistently create namespaces
ALIAS
targets in subprojects. Only link to namespaced GeoModel targets in subprojects- Avoids need to check whether a project is built in a full GeoModel build or standalone.
- Make
ExternalProject
targets for JSON, Eigen, and Xerces-C dependencies of the internal imported targets- Avoids need to use
add_dependencies
on GeoModel targets that link to these. They simply link to the namespaced imported target, e.g.nlohmann_json::nlohmann_json
and don't need to worry if this is coming from the builtin on the system.
- Avoids need to use
- Fully implement the
FullSimLight
target as anINTERFACE
target so plugins just need to link to it. - Build the core of the FullSimLight application as an
OBJECT
library.- Applications just link to this instead of recompiling the sources each time for their own use.
- General simplifications:
- Keep all build commands for a target as close together as possible for clarity
- Check subproject status using
CMAKE_CURRENT_SOURCE_DIR
instead of an artificialproject
command. - Remove dead code or no longer required/supported modules (e.g.
Geant4_USE_FILE
)
These have been tested locally on macOS with as many combinations of build options as I can enable and appears fine. CI is more than likely to highlight issues, but let's see what it throws up.
Merge request reports
Activity
Hi @bmorgan , thanks very much for this MR! To me it looks fine, I'd ask @boudreau and @rbianchi also to have a look at it before merging. Also Ben, could you please solve the conflicts that appeared in the meantime?
Many thanks, Marilena
Edited by Marilena Bandieramonte- Resolved by Joseph Boudreau
- Resolved by Joseph Boudreau
Once again, thanks for this modernization, @bmorgan , which is for sure very useful.
However, I must say that now I recall why we still sticked to workaround solutions for earlier CMake versions: the main reason was the "official" version on Lxplus, which is also the one shipped with CC7 by default.
So, I see two ways here:
- We keep the current setup, which works on earlier CMake versions too, and we postpone the modernization to when CERN officially will move to Lxplus8, which provides CMake 3.20.2
- We update our setup with your MR, but we have to warn people using the default Lxplus to either: a) set up a bare ATLAS environment with
--cmakeversion
option ofasetup
; b) connect to Lxplus8 instead of the default Lxplus, which still points to Lxplus7
I would suggest @boudreau and @tsulaia should decide what to do.
Best, -- Ric.
Edited by Riccardo Maria Bianchi
- Resolved by Joseph Boudreau
Ric, does this mean (for example) that we could not cut a version of our code for use as an "External" in an athena build, if these changes were to be adopted?
added 36 commits
-
db5e6fd6...d877c836 - 23 commits from branch
GeoModelDev:master
- f304c3c8 - Bump minimum CMake for core packages to 3.16
- 3cd8f410 - Rationalize top level CMake script
- b17e3d87 - Simplify use of COLOR_DEFS output
- 4b9cb47e - Rationalize FullSimLight cmake script
- 3d098064 - Simplify build/use of builtin/system JSON
- 26d26704 - Simplify build/use of builtin/system XercesC
- 1cc28ca0 - Simplify build/use of builtin/system Eigen
- cd4f5c34 - Always link to namespaced CMake targets
- 6dd1186f - Implement FullSimLight alias/exported target
- f23434b2 - Streamline build of FullSimLight
- 1b7c973a - Simplify build of GeoModelG4
- 6da896ee - Simplify FullSimLight CMake script
- 7602726d - Only set color strings if COLOR_DEFS is true
Toggle commit list-
db5e6fd6...d877c836 - 23 commits from branch
Sorry to bump this, but is there anything else I need to do so this can be merged? It would be useful to have this in
master
for the next step addressing @akraszna's suggestion to useFetchContent
, and also to begin the testing of GPU offload with AdePT and Celeritas!Hi @bmorgan , this MR looks fine on my side. I'd ask @rbianchi , @boudreau , @tsulaia a final confirmation. Thanks, Marilena
Edited by Marilena Bandieramontementioned in commit 96396bed