Distributed support#47
Open
constracktor wants to merge 58 commits into
Open
Conversation
constracktor
force-pushed
the
distributed
branch
8 times, most recently
from
July 6, 2026 15:44
bbb69cb to
b9b9c29
Compare
from 6b575523ce838fc13517d1a8021ce4883efc29c1
This is a separate commit for git's rename tracking
Now that our headers are properly namespaced, there's no need to prefix their filenames with gp_ or end them with _c to avoid name clashes with library users.
They're not costly in terms of workflow minutes so we can just do that.
in lieu of std::vector<T> for tiles of type T. The advantage of this is: - tiles are easily HPX-serializable and we can put them into HPX components - we can perhaps later add support for automatic GPU upload
Powered by HPX's performance counter library. Since this library is only built if networking != none, guard against it being missing.
Quite a few functions took `int` parameters just to cast them to `std::size_t` everywhere.
Extends our performance counter to track #calls and runtime.
Algorithms supporting different schedulers are templates now. Consequently, they had to be moved from .cpp to .hpp
This is required because newer CMake versions don't support cmake_minimum_required with minimum versions <= 3.5
Based on our shared-memory experiments.
named_make_tile was passing hpx::find_here() instead of sched.localities_[on], causing all tile creation to happen on the calling locality regardless of the scheduling hint. The sched and on parameters were also silenced with /* */ comments. named_async was passing the HPX launch policy as the callable to hpx::annotated_function instead of the action, matching neither the two-argument form (callable, name) nor the signature used by named_dataflow and named_make_tile. Both bugs meant the distributed scheduler never actually distributed any work: all tiles and all async calls landed on the local node. fix(distributed): tile_holder<T> was hardcoding double instead of T The tile_holder<T> class template declared constructor, get_data(), set_data(), and the data_ member all with mutable_tile_data<double> instead of mutable_tile_data<T>. The template parameter T was entirely unused. Any future non-double instantiation would silently widen or truncate data without a compile error.
The constructor validated that the requested id is within range but discarded the selected sycl::device without storing it. create() then called sycl::queue(sycl::gpu_selector_v) which always targets GPU 0, ignoring the id the user requested. Multi-GPU SYCL configurations were silently routing all work to GPU 0. Fix: store the selected sycl::device in selected_device_ during construction, and use it in create() instead of gpu_selector_v. Also widen the catch from sycl::exception to std::exception so that the std::runtime_error thrown on invalid device id is actually caught and printed before being re-thrown, rather than propagating silently to std::terminate.
CUDA_GPU::i_stream and SYCL_DEVICE::i_queue were plain int/size_t incremented with i_stream++ / i_queue++. Two concurrent HPX tasks sharing the same device could race on the counter, producing a data race (UB) and potentially assigning two tasks to the same stream/queue. Changed to std::atomic<int> / std::atomic<std::size_t> with explicit copy constructors and copy assignment operators to restore copyability. Also removed the dead hpx::shared_future<sycl::queue> f_dot_diag_syrk in predict_with_full_cov which consumed a next_queue() slot on every inner loop iteration without ever being used.
command -v icpx --version always returns non-zero because --version is interpreted as a second command name (not a flag) by command -v. This broke the AMD/Intel SYCL compiler detection paths silently. Remove --version so the check is command -v icpx. GPRAT_SYCL_CUDA_PATH was declared with option() which creates a BOOL CMake cache variable. Filesystem paths in BOOL variables are fragile: cmake-gui shows a checkbox, and cmake --fresh may normalize the value. Changed to set(... CACHE PATH ...) matching the adjacent GPRAT_SYCL_NVIDIA_ARCH which correctly uses CACHE STRING.
…holder leak set_async: add [[nodiscard]] so ignoring the returned handle is a compile warning. The original handle's generation_ is intentionally not updated (functional style); callers must assign the future result back. set_tile_data_async: move cache_.insert into a .then() continuation so the cache is only populated after the remote write confirms success. The previous code inserted before the async write, leaving a stale cache entry if the network call failed. managers_ vector in tile_handle: changed from std::vector copied once per tile to std::shared_ptr<std::vector> shared across all tile_handles created from the same dataset. Eliminates O(num_tiles × num_localities) hpx::id_type copies at dataset construction time. Added a shared_ptr overload to the constructor so create_tiled_dataset can pass one shared allocation. local_manager(): renamed from get_local_manager() and added a per-handle cache (cached_manager_). The hpx::get_ptr<tile_manager>(sync, id) call now happens once per tile_handle rather than on every get()/set_async() in O(tiles²) algorithm loops. cached_manager_ is mutable and excluded from serialization. create_tiled_dataset: stop outer loop as soon as num_tiles is satisfied (was calling holders[i].get() and discarding all returned tile_holder HPX components for over-allocated localities, leaking remote memory). Add an explicit error if total slots < num_tiles.
Both files had Windows-style CRLF line endings and a UTF-8 BOM, inconsistent with every other source file in the repository.
- forward_solve_tiled: save x_k before inner loop so GEMV always uses
the solved tile, not the overwritten b vector (fixed predict tests 72-74)
- move_lower_tiled_matrix_to_host / copy_tiled_vector_to_host_vector:
use a single queue instead of N default-constructed queues
- gprat.cpp GP::cholesky: skip mutable_tile_data construction for empty
upper-triangle tiles to avoid allocate_tile_data(0) -> bad_alloc (tests 76-77)
- dot(): use sycl::malloc_shared instead of malloc_device so the scalar
result is readable from the host without an explicit copy
- compute_loss(): copy diagonal tile to host vector before reading it;
dereferencing a device pointer on the CPU caused an indefinite hang (test 75)
- compute_loss_tiled(): replace deferred hpx::dataflow([&](){...}) loop
(k captured by reference, .get() inside HPX tasks) with a plain
synchronous loop; fixes the infinite hang and simplifies the logic
- execute.cpp: add --use_sycl / --use_gpu as recognized program options
and wire the SYCL execution path so the example runs on the GPU
All 9 SYCL tests (71-79) now pass. gprat_cpp --use_sycl produces correct
timing output.
Fix python cuda fix(compile): initialize SYCL vars to defaults and fix dead elif GPRAT_SYCL_NVIDIA/AMD/INTEL and HIP_TARGETS were only set inside host-specific branches, causing empty strings to be passed to CMake on other hosts. Add safe defaults before the host-detection block. Also replace the always-true `elif [[ "$2" != "cpu" ]]` with `else`.
- Match parameter signature: $1=cpu/cuda/sycl, $2=nvidia/amd/intel (drop separate mkl param; remove unused CMAKE_PREFIX_PATH from $3) - Fix GPU flag: --use_cuda → --use_gpu (was crashing with "unrecognized option" before even reaching GPU computation) - Fix cmake paths: use SCRIPT_DIR to anchor GPRat_DIR correctly (was resolving to build/run_gprat_cpp/lib/ instead of lib/) - Simplify build dir: one level (build/) instead of build/run_gprat_cpp/ - Add LD_LIBRARY_PATH setup per host (hpx, openblas, mkl via spack) - Add simcl1n4 CPU-only host with GPU guard - Add icpx sourcing for SYCL on simcl1n1/simcl1n2 (matching compile_gprat.sh) - Set GPRAT_WITH_CUDA/SYCL in cmake call to avoid macro redefinition - Read config.json via relative path (../config.json from build/) - Use GPRAT_LIB_SUFFIX instead of ADD for lib64 variant debug: add runtime diagnostics to cpp example to trace DGEMM bug fix(examples): fix n_test=n_train, auto-install correct backend, fix Python cwd - execute.cpp: set n_test=n_train inside the loop so tiles are always square (non-square tiles are unsupported on GPU); remove debug prints - run_gprat_cpp.sh: auto-install matching GPRat backend before building so lib/ always reflects the correct backend; fix NFS-resilient build dir cleanup - run_gprat_python.sh: auto-install matching backend and copy the correct .so from bindings/; cd to SCRIPT_DIR before running execute.py All 6 backends (C++ and Python × cpu/cuda/sycl) now pass. Adjust examples Adjust to amd gpu
- gp_algorithms_actions.hpp: gen_tile_zeros registered under wrong HPX label "gen_tile_output" (copy-paste); fixes remote action dispatch - compile_gprat.sh: fix simcl1n3 CPU branch indentation — inner spack check was outside the then-body, so GPU setup always ran for cpu builds - compile_gprat.sh: guard simcl1n1 SYCL ONEAPI_COMPILER_ROOT fallback with `command -v icpx` before calling `which icpx` (set -e safety)
…chema In v0.4.0 predictions were already run before optimize(), making the _no_optimize suffix redundant. GPU test now stores results in sum/full/pred matching the CPU test and the original output.json schema. The baseline will auto-regenerate on next test run since the current output.json has empty sum/full/pred arrays (stale detection triggers).
- Fix duplicate 'To run GPflow reference' header → GPyTorch - Replace -gpu presets with -cuda/-sycl variants - Update CMake options table: GPRAT_USE_MKL→GPRAT_ENABLE_MKL, add GPRAT_ENABLE_TESTS and GPRAT_WITH_DISTRIBUTED - Document new 5-param compile_gprat.sh signature - Update run_gprat_cpp.sh and run_gprat_python.sh usage examples - Mention fp32/fp64, NUMA allocator, perf counters, distributed support
CTestCostData.txt is a build artifact generated by CTest at runtime. It should not be version-controlled.
Fixes formatting violations that would fail the lint CI check.
…APEX HPX built with +static and instrumentation=apex (as used by CI's spack env) embeds APEX's private zlib/rapidjson/otf2 dependencies into HPXTargets.cmake by bare name instead of as proper exported targets. With no matching target in the consuming project, CMake falls back to raw -l<name> linker flags, which can never resolve: real zlib produces libz (not libzlib), and rapidjson is header-only with no library file at all. Define compatibility targets for these exact names so the lookup succeeds before CMake degrades to a linker flag.
…benchmark The built-in reference-comparison check only ever validated one exact configuration (n_train=128, n_tiles=4) matching test/output_correctness.cpp, so it spuriously failed for any other size in a --start/--end sweep and silently returned exit code 0 on failure. Remove the check (correctness is already covered by the dedicated unit test), decouple the example from test/src/test_data.hpp, Catch2, and Boost.Json in the process, fix the exit code, add real Init_time/Localities columns to the timings CSV, and add a run script plus README section matching the other examples' conventions.
- Guard against silently misaligned CSV columns when --timings_csv points at a file written by an older binary version (different header) by validating the existing header before appending, instead of appending mismatched rows. - Drop stray "*/" comment artifact from the --opt_iter help string. - run_gprat_distributed.sh: only pass -DHPX_DIR when actually set (was unconditionally passed empty on every host but sven0/sven1); dedupe the simcl1n1-4 hostname check into a shared helper.
Rename --timings_csv to --output_csv and default it to <example dir>/output.csv (baked in via a compile definition, like gprat_cpp's GPRAT_CPP_CONFIG_PATH) instead of "timings.csv" resolved against the current working directory, matching the other examples' convention of writing output next to their own sources.
The default gprat_cpu_gcc Spack environment builds HPX with networking=none, which rejects --hpx:localities outright, so multi-locality runs of the distributed benchmark were previously undocumented and unbuildable through run_gprat_distributed.sh. Add a gprat_cpu_gcc_dist Spack environment (networking=tcp, OpenBLAS-only) and default the script to it (GPRAT_DIST_MULTI_LOCALITY=1, set to 0 to opt back into the old single-locality build), building into a separate build/release-linux-dist directory to avoid poisoning the CMake cache shared with the default build. For each locality count in GPRAT_DIST_LOCALITIES (default "1 2 4"), the script now spawns and waits for that many --hpx:node processes itself in one invocation. Also comment out the full HPX runtime config dump the example printed on every run (~300 lines of AGAS/logging/thread-pool noise per locality), which drowned out the actual benchmark timings. Verified locally at 1/2/4 localities.
There was no automated coverage for gprat_distributed's multi-locality support, only manual verification. Add GPRAT_TEST_MULTI_LOCALITY (off by default, since it needs an HPX build with networking enabled, which the default gprat_cpu_gcc Spack environment doesn't have) registering CTest entries that launch gprat_distributed across 1/2/4 localities via a helper script and fail if any locality's process exits non-zero. Verified passing against the gprat_cpu_gcc_dist environment, and verified the test correctly fails against a broken binary path.
ctest -j runs tests concurrently by default. Each multi-locality test's node 0 binds HPX's default AGAS/parcel TCP port on localhost, so running two of these tests at once makes the second's bind fail with "Address already in use", crashing that test's node 0 and leaving its peer processes hanging until they hit the 120s TIMEOUT (reproduced with `ctest -R multi_locality -j 3`, which timed out 2 of 3). Mark them RUN_SERIAL so ctest never overlaps them, regardless of -j. Verified: `ctest -R multi_locality -j 3` now passes all three, and the full suite passes under `ctest -j 8`.
…t exit codes The multi-locality tests added in f3dce43 only checked that gprat_distributed exits 0. Considered running GPRat_test_output_correctness itself across localities for a stronger check, but gprat::GP always uses tiled_scheduler_local, which is locality-oblivious -- it never touches the gp_*_actions.cpp distributed dispatch regardless of --hpx:localities, so that would add no real coverage. Add GPRat_test_distributed_output_correctness instead: a small non-Catch2 binary that calls the same free functions (cpu::cholesky, cpu::optimize, ...) with tiled_scheduler_sma -- the scheduler examples/gprat_distributed actually uses -- and compares the results to the same data/data_1024/output.json baseline GPRat_test_output_correctness checks. It runs under hpx::init/hpx_main (hpx_main only executes on locality 0 by default, which is exactly the gating needed here) via the generalized run_distributed_multi_locality.sh, registered for 1/2/4 localities. Factored load_or_create_expected_results out of output_correctness.cpp into test_data.hpp so both binaries share it. Verified: all 3 pass and actually compare (not skip) against the baseline, and correctly fail (exit 1, with a diff) when the baseline is deliberately corrupted.
An adversarial review of 94f8c46 found that GPRat_test_distributed_output_correctness hardcoded --data_root to CMAKE_SOURCE_DIR/data while GPRat_test_output_correctness resolves the data directory via the GPRAT_ROOT env var (falling back to "../data"). If GPRAT_ROOT were ever set to something other than the source tree, the two test families would silently read/write different output.json baselines and stop cross-validating against each other, defeating the point of sharing one baseline. Factor get_data_directory() into test_data.hpp (parameterized by each binary's own fallback) so both binaries agree whenever GPRAT_ROOT is set. The distributed binary's own fallback is now a compile-time GPRAT_TEST_DATA_DIR definition instead of a --data_root CLI option, since it has no Catch2 CLI parsing to route around. Verified: full suite (77 tests) still passes, including a run with GPRAT_ROOT unset (both fall back to the source tree) and a manual check that -V output for both test families still shows a real baseline comparison, not a skip.
Fixes the "Code linting" CI failure on this branch (https://github.com/SC-SGS/GPRat/actions/runs/28666890010): clang-format flagged recent edits in main.cpp and distributed_output_correctness.cpp, and cmake-format flagged cmake/hpx-apex-compat.cmake (pre-existing, unrelated to this branch's recent commits) plus reflowed the CMakeLists.txt changes from this branch into its canonical wrapping. No behavior change; reproduced and verified locally (spack llvm@18.1.8's clang-format as clang-format-18, pip cmakelang) with the exact `cmake --build build-fmt --target check-clang-format`/`check-cmake-format` commands lint.yml runs, both now pass, and the full test suite (77 tests) still passes after applying the patches.
Guard the tiled_dataset.hpp include behind GPRAT_WITH_DISTRIBUTED so a plain CPU build compiles against an HPX built with networking=none.
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
2 participants
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.
This PR extends the GPRat::core target to: