Closed Yurlungur closed 2 years ago
Another consideration: My current draft does not support "in-tree" Kokkos builds of the spiner tests. However, if Kokkos is being built in-tree in a larger project, then that still works. Is there a desire to support "in-tree" builds for the tests?
I also took this as an opportunity to invite @jhp-lanl and @mauneyc-LANL to the repo.
Another suggestion by @mauneyc-LANL : Add a spiner::spiner
target, which includes the spiner::flags
and spiner::libs
targets.
With @mauneyc-LANL 's suggestions, I'm removing the WIP. I will merge once I've implemented them, if I don't hear from anyone else.
With two approvals and tests passing, I'm going to merge this. That said, @dholladay00 I am interested in your thoughts. If you get the chance to look, let me know what you think.
took a brief look and everything looked ok to me. currently do we install this (when installing singularity-eos) just by copying headers, did you test if installation behavior needed to change @Yurlungur ?
took a brief look and everything looked ok to me. currently do we install this (when installing singularity-eos) just by copying headers, did you test if installation behavior needed to change @Yurlungur ?
Thanks, @dholladay00 ! In singularity-eos
, yes, all we do is copy the headers. I didn't change the cmake
build system for singularity-eos
to reflect the changes in spiner
. What will need to change is we'll need to add spiner
as a subdirectory, but then things should just work (I hope).
PR Summary
In this PR, I move the build system to cmake. I do so mainly to make
spiner
more consistent with, e.g., the best-practices we've built up in singularity-eos. In practice, this means the following changes:Makefile
s intest
have been removedspiner
subdirectory, just for better code organization. This also let's me make aninstall
target for spiner, as well as modify the include paths to be#include <spiner/filename.hpp>
.Catch2
is now dynamically fetched. The submodule has been removed.CMakeLists.txt
, which I have attempted to do. There are some gotchas. I found it wouldn't build if I installed the default spack variant of Kokkos. I neededkokkos~shared
.make format
in the build directory to auto-format the code. I did so, which means there are significant diffs to some files. In particularports-of-call/portability.hpp
,ports-of-call/portable-arrays.hpp
, andspiner/databox.hpp
all have been autoformatted, but had no meaningful changes otherwise. @chadmeyer please let me know if this is a problem for theports-of-call
files. It can be undone.My hope is this will make it easier to integrate
spiner
with spack, with our CI, and with thesingularity-eos
build system we've constructed. That said, I could be wrong on this. I was disappointed to find that the newCMakeLists.txt
are like 3x times more lines than the oldMakefile
s, which makes me wonder. It's also a breaking change with downstream, since the directory structure changed. (Of course, the downstream fixes should be straightforward.)Due to my uncertainty, I've marked this as WIP until I can get input from stakeholders, in particular, I'm hoping to hear from at least a few of:
spiner
andsingularity-eos
Please take a look and let me know what you think.
PR Checklist