Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/vt-lb/comm/vt/proxy_wrapper.impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ void ProxyWrapper<ProxyT>::reduceAnonCb(vt::collective::ReduceTMsg<T>* msg, Redu
} else {
using ValT = typename T::value_type;
static_assert(std::is_trivially_copyable_v<ValT> || std::is_arithmetic_v<ValT>, "Reduce value must be trivially copyable");
std::memcpy(ctx->out_ptr, std::addressof(val), sizeof(ValT) * std::max<std::size_t>(1, ctx->count));
std::memcpy(ctx->out_ptr, std::addressof(val.at(0)), sizeof(ValT) * std::max<std::size_t>(1, ctx->count));
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] While std::addressof(val.at(0)) is functionally correct, using val.data() would be more idiomatic for obtaining a pointer to the container's underlying data. For example: std::memcpy(ctx->out_ptr, val.data(), sizeof(ValT) * std::max<std::size_t>(1, ctx->count));

Suggested change
std::memcpy(ctx->out_ptr, std::addressof(val.at(0)), sizeof(ValT) * std::max<std::size_t>(1, ctx->count));
std::memcpy(ctx->out_ptr, val.data(), sizeof(ValT) * std::max<std::size_t>(1, ctx->count));

Copilot uses AI. Check for mistakes.
}
ctx->done.store(true, std::memory_order_release);
}
Expand Down
186 changes: 186 additions & 0 deletions tests/unit/comm/test_comm.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,186 @@
/*
//@HEADER
// *****************************************************************************
//
// test_comm.h
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The header comment indicates this file is named "test_comm.h", but the actual filename is "test_comm.cc". The comment should be corrected to match the actual filename.

Suggested change
// test_comm.h
// test_comm.cc

Copilot uses AI. Check for mistakes.
// DARMA/vt-lb => Virtual Transport/Load Balancers
//
// Copyright 2019-2024 National Technology & Engineering Solutions of Sandia, LLC
// (NTESS). Under the terms of Contract DE-NA0003525 with NTESS, the U.S.
// Government retains certain rights in this software.
//
// Redistribution and use in source and binary forms, with or without
// modification, are permitted provided that the following conditions are met:
//
// * Redistributions of source code must retain the above copyright notice,
// this list of conditions and the following disclaimer.
//
// * Redistributions in binary form must reproduce the above copyright notice,
// this list of conditions and the following disclaimer in the documentation
// and/or other materials provided with the distribution.
//
// * Neither the name of the copyright holder nor the names of its
// contributors may be used to endorse or promote products derived from this
// software without specific prior written permission.
//
// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
// AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
// IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
// ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
// LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
// CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
// SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
// INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
// CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
// ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
// POSSIBILITY OF SUCH DAMAGE.
//
// Questions? Contact [email protected]
//
// *****************************************************************************
//@HEADER
*/

#include <gtest/gtest.h>

#include "test_parallel_harness.h"
#include "test_helpers.h"

#include <atomic>
#include <vector>
#include <numeric>
#include <tuple>
#include <type_traits>

namespace vt_lb { namespace tests { namespace unit {

template <comm::Communicator CommType>
struct TestCommBasic : TestParallelHarness<CommType> {
struct TestObject {
int calls = 0;

void ping(int v) { calls += v; }
};

// Member function pointers for send
static constexpr auto fn_ping = &TestObject::ping;

using HandleT = typename CommType::template HandleType<TestObject>;

// helper to register object and return handle
HandleT makeHandle(TestObject* obj) {
return this->comm.template registerInstanceCollective<TestObject>(obj);
}

// helper to drain progress
void progressUntil(std::function<bool()> pred, int max_iters = 10000) {
int it = 0;
while (!pred() && it++ < max_iters) {
this->comm.poll();
}
}
};

TYPED_TEST_SUITE(TestCommBasic, CommTypesForTesting, CommNameGenerator);

TYPED_TEST(TestCommBasic, test_init_finalize_and_basic_props) {
auto& the_comm = this->comm;

SET_MIN_NUM_NODES_CONSTRAINT(2);

// Basic size/rank API should work
ASSERT_GE(the_comm.numRanks(), 1);
ASSERT_GE(the_comm.getRank(), 0);
ASSERT_LT(the_comm.getRank(), the_comm.numRanks());
// Poll should be callable
(void)the_comm.poll();
}

TYPED_TEST(TestCommBasic, test_send_and_poll_dispatch) {
auto& the_comm = this->comm;

SET_MIN_NUM_NODES_CONSTRAINT(2);

typename TestFixture::TestObject obj{};
auto handle = this->makeHandle(&obj);

// Send to neighbor in a ring
int self = the_comm.getRank();
int n = the_comm.numRanks();
int dest = (self + 1) % n;

// Each rank sends to its right neighbor; the left neighbor sends to us with payload (left+1)
the_comm.template send<TestFixture::fn_ping>(dest, handle, self + 1);

// Expected value received from left neighbor
int left = (self - 1 + n) % n;
int expected = left + 1; // equals self for self>0, equals n for self==0

// Progress until our local object receives from our left neighbor
this->progressUntil([&] { return obj.calls == expected; });
EXPECT_EQ(obj.calls, expected);
}

TYPED_TEST(TestCommBasic, test_reduce_sum_int_single) {
auto& the_comm = this->comm;

SET_MIN_NUM_NODES_CONSTRAINT(2);

typename TestFixture::TestObject obj{};
auto handle = this->makeHandle(&obj);

int const root = 0;
int send = 1;
int recv = 0;

handle.reduce(root, MPI_INT, MPI_SUM, &send, &recv, 1);

if (the_comm.getRank() == root) {
// Sum over all ranks of 1
EXPECT_EQ(recv, the_comm.numRanks());
}
}

TYPED_TEST(TestCommBasic, test_reduce_max_double_array) {
auto& the_comm = this->comm;

SET_MIN_NUM_NODES_CONSTRAINT(2);

typename TestFixture::TestObject obj{};
auto handle = this->makeHandle(&obj);

int const root = 0;
// each rank contributes two doubles, make max depend on rank
std::array<double,2> send{{double(the_comm.getRank()), double(the_comm.getRank() + 1)}};
std::array<double,2> recv{{0.0, 0.0}};

handle.reduce(root, MPI_DOUBLE, MPI_MAX, send.data(), recv.data(), int(send.size()));

if (the_comm.getRank() == root) {
EXPECT_DOUBLE_EQ(recv[0], double(the_comm.numRanks() - 1));
EXPECT_DOUBLE_EQ(recv[1], double(the_comm.numRanks()));
}
}

TYPED_TEST(TestCommBasic, test_reduce_sum_float_array) {
auto& the_comm = this->comm;

SET_MIN_NUM_NODES_CONSTRAINT(2);

typename TestFixture::TestObject obj{};
auto handle = this->makeHandle(&obj);

int const root = 0;
std::vector<float> send(4, 1.0f); // each rank contributes 4 ones
std::vector<float> recv(4, 0.0f);

handle.reduce(root, MPI_FLOAT, MPI_SUM, send.data(), recv.data(), int(send.size()));

if (the_comm.getRank() == root) {
for (auto v : recv) {
EXPECT_FLOAT_EQ(v, float(the_comm.numRanks()));
}
}
}

}}} // end namespace vt_lb::tests::unit
2 changes: 1 addition & 1 deletion tests/unit/test_helpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ inline std::string getUniqueFilename(const std::string& ext = "") {
*/
#define SET_MIN_NUM_NODES_CONSTRAINT(min_req_num_nodes) \
{ \
auto const num_nodes = comm.numRanks(); \
auto const num_nodes = this->comm.numRanks(); \
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SET_MAX_NUM_NODES_CONSTRAINT macro on line 118 uses comm.numRanks() without the this-> prefix, while SET_MIN_NUM_NODES_CONSTRAINT was updated to use this->comm.numRanks(). For consistency, this macro should also be updated to use this->comm.numRanks().

Copilot uses AI. Check for mistakes.
if (num_nodes < min_req_num_nodes) { \
GTEST_SKIP() << fmt::format( \
"Skipping the run on {} nodes. This test should run on at least {} "\
Expand Down