Forest fire - #167
Open
jacob-moore22 wants to merge 43 commits into
Open
Conversation
jacob-moore22
marked this pull request as ready for review
August 26, 2026 16:02
This reverts commit 5414de8.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pure-CMake build, Kokkos 5.2.1, swappable precision, host-side parallel macros
Summary
Replaces MATAR's bash build system with a self-contained CMake build so ELEMENTS and Fierro can consume
MATAR + Kokkos via
FetchContent,add_subdirectory, orfind_package. Trilinos support is removed(retired under
legacy/). On top of that: the bundled Kokkos moves 4.3.0 → 5.2.1 (and the languagestandard to C++20), a compile-time-swappable floating-point precision system lands, host-side parallel
macros are added, and
set_valuesis reimplemented onKokkos::deep_copy.Build system
One root
CMakeLists.txtdefines the header-onlymatarINTERFACE target (+matar::mataralias). Alldefines (
HAVE_KOKKOS,HAVE_MPI, backendHAVE_*) and link dependencies attach to the target, soconsumers inherit them through
target_link_libraries(app matar::matar).MATAR_ENABLE_KOKKOSMATAR_ENABLE_MPIMPICArrayKokkos,CommunicationPlanMATAR_ENABLE_GPU_AWARE_MPIMATAR_USE_EXTERNAL_KOKKOSMATAR_BUILD_EXAMPLES/_TESTS/_BENCHMARKSMATAR_REAL/MATAR_HIGH_REAL/MATAR_LOW_REALdoublecontrol; external Kokkos is explicit opt-in. If a parent project already provides
Kokkos::kokkos,MATAR uses it. Backends use standard
Kokkos_ENABLE_*/Kokkos_ARCH_*— the--kokkos_build_typeenumis gone (this also fixes
cuda_mpibuilds missingMatar_CUDA_BUILD).CMakePresets.jsonreplaces the script's backend matrix:serial,openmp,pthreads,cuda,hip,*-mpi,*-debug—cmake --preset openmp && cmake --build --preset openmp && ctest --preset openmp.find_package(Matar)works from an installed prefix; bundled Kokkos installsalongside so
find_dependency(Kokkos)resolves. LegacyMatar_ENABLE_KOKKOS/MPImap with a deprecationwarning;
Matar_ENABLE_TRILINOSis a hard error pointing atlegacy/.configure-time shell script to
ExternalProject_Add.gtest_discover_tests(... DISCOVERY_MODE PRE_TEST), so builds never execute thetest binary. MPI suite runs under
mpirun -n 4.Kokkos 5.2.1 + C++20
Submodule bumped 4.3.0 → 5.2.1;
target_compile_featuresis nowcxx_std_20. Source changes for theKokkos 5 API, plus suppression of incorrect nvcc warnings in the test sources.
Swappable precision (
src/include/precision.h, new)Three tier names whose meaning is fixed for the whole build:
real_tMATAR_REALhigh_real_tMATAR_HIGH_REALlow_real_tMATAR_LOW_REALhalf/bfloat16 map to the Kokkos types (native 16-bit on CUDA/HIP/SYCL, float-backed elsewhere — reported
by
MATAR_FP16_IS_EMULATED/MATAR_BF16_IS_EMULATED); quad is__float128, host backends only, needsKokkos_ENABLE_LIBQUADMATH. Non-Kokkos builds support double/float only. MPI types map each tier to theright
MPI_Datatype.Host-side parallel macros
FOR_ALL_HOST,DO_ALL_HOST,RUN_HOST,FOR_REDUCE_{SUM,MAX,MIN,PRODUCT}_HOST,DO_REDUCE_{SUM,MAX,MIN}_HOST, their_CLASSvariants, andMATAR_FENCE_HOST()/MATAR_FENCE_DEVICE(). These run onKokkos::DefaultHostExecutionSpaceand capture by reference, sohost-only objects (
std::string,std::vector) work inside them and they can sit in a private memberfunction. On a no-Kokkos build they alias the serial macros.
The supported interop pattern is host macros on the
.host()side of a dual type, thenupdate_device()(
TestMacrosHost.DualTypeHostSide). Launching a device kernel and host work back-to-back without anintervening fence composes correctly (
TestMacrosHost.ConcurrentHostAndDeviceWork), but nothing assertsthe two actually overlap, and host/device macros over the same buffer is still a data race.
set_valuesKokkos::deep_copy(array_, val)instead of a hand-rolledparallel_for.clear_sync_state()— Kokkos has nodeep_copy(DualView, scalar),and marking both sides modified trips DualView's concurrent-modification abort. Both sides now hold the
value with no
update_host()required.KOKKOS_INLINE_FUNCTION ... const, so a View constructed inside a kernel can fillitself there (issue View set_values calls should be standard for loop #146).
constis required becauseKOKKOS_LAMBDAcaptures by value into anon-mutable closure; without it the captured view is const and the call fails to compile.
test_set_values_deep_copy.cpppins the contract the per-type tests could not (they all calledupdate_host()before checking.host()).Consuming MATAR
add_subdirectoryand installedfind_package(Matar)work identically; the plainmatartarget name iskept. The tutorials use exactly this pattern and serve as living tests of it.
Bug fixes surfaced by the migration
matar.h:mpi_types.hincluded only underHAVE_MPI.macros.hserial/no-Kokkos path (never previously CI-tested):_CLASSaliases pointed at nonexistentnames; added missing serial
RUN/RUN_CLASS,FOR_REDUCE_PRODUCT, 2D/3Dreduce_prod.DynamicRaggedDownArrayKokkos::dims(): missing return path (UB in Release fordims(2)) and wrongassert bound.
CArrayDevice_benchmark.cpp: used the long-removedREDUCE_SUMname.examples/matar_mpi.cpp: signed loop bounds (multi-dimFOR_ALLbrace-initializes a signedMDRangePolicy).Formatting + CI
.clang-format(Google base, 4-space, 150 cols,SortIncludes: Never) plusformatting/matar-format.py, a post-processor producing the canonical parallel-macro layoutclang-format cannot. Applied across
src/include/,solvers/,examples/,tutorial/,test/;macros.his excluded via.clang-format-ignore.test.ymlruns the preset matrix (serial/openmp × debug/release on Ubuntu + Mac, serial-mpi-debug withthe mpirun suite).
cmake.ymlcovers no-Kokkos, bundled-Kokkos + install round-trip with a downstreamfind_package(Matar)smoke test, and external-Kokkos.Verification
cuda-mpi: 317/317 tests pass, including thempirun -n 4suiteadd_subdirectoryconsumer projects compile and runWhat this may break downstream
Anyone tracking
main(ELEMENTS, Fierro) should expect the following:matarexportscxx_std_20as an INTERFACE feature, so it propagates toevery consumer. Anything pinned to C++17 will fail to configure or hit new standard-conformance errors.
u_intis gone. It was a globalusing u_int = unsigned int;inaliases.hand no header definesit any more. Any downstream use fails to compile — the fix is
unsigned intor a local typedef.real_tmoved toprecision.hand is no longer unconditionallydouble. It is still availableglobally and still defaults to
double, but a build that setsMATAR_REALchanges its meaningeverywhere. Code that assumes
real_t == double(MPI_DOUBLEliterals,%lfformats, reinterpretcasts, ABI across a library boundary) is now fragile.
be removed.
Kokkos_ENABLE_CUDA_LAMBDAis dropped (on by default in 5.x). Downstream projectsproviding their own Kokkos must supply 5.x, since MATAR headers now compile against that API.
tpetra_wrapper_types.h,Tpetra_LRMultiVector_{decl,def}.hpp,Tpetra_LR_WrappedDualView.hpp,mapped_mpi_types.h,partition_map.h,mpi_types_old.h,communication_plan_old.hare deleted fromsrc/include/. Any#includeof them breaks;Matar_ENABLE_TRILINOSis now a hard CMake error.scripts/build-matar.shand friends, now underlegacy/). CI ordeveloper workflows invoking them must move to the presets.
set_valueson Dual types now populates both sides. Code that relied on the host side beingstale after
set_values(e.g. asserting a pending sync, or callingupdate_host()to detect achange) will observe different behavior. The values themselves are correct either way.
set_valuesis nowconst. Additive for callers, but an override or an explicitvoid (View::*)(T)member-pointer will no longer match the signature.Matar_ENABLE_KOKKOS/Matar_ENABLE_MPIstill map to the newnames but emit a deprecation warning; builds with
-Werror-style CMake policies should update.Follow-up work (not in this PR)
(Build error with Cuda backend and C++ 20 when relocatable device code is enabled kokkos/kokkos#7779)