Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

2096 listInsertHere with an empty list fails check #2105

Merged
merged 3 commits into from
Mar 27, 2023
Merged

2096 listInsertHere with an empty list fails check #2105

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
09ee6a4
#2096: collection: allow listInsertHere with empty list
nlslatt Mar 14, 2023
7a1d145
#2096: tests: fix duplicate elm indices
nlslatt Mar 27, 2023
c58f5bc
#2096: tests: add test for empty listInsertHere
nlslatt Mar 27, 2023
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
5 changes: 4 additions & 1 deletion src/vt/vrt/collection/param/construct_params.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,7 @@ struct ConstructParams {
*/
template <typename Iter>
ThisType&& listInsertHere(Iter begin, Iter end) {
did_list_insert_here_ = true;
for (auto&& iter = begin; iter != end; ++iter) {
list_insert_here_.emplace_back(std::move(*iter));
}
Expand Down Expand Up @@ -388,7 +389,7 @@ struct ConstructParams {
has_bounds_ or
(not has_bounds_ and bulk_inserts_.size() == 1) or
(not has_bounds_ and
(list_inserts_.size() > 0 or list_insert_here_.size() > 0)),
(list_inserts_.size() > 0 or did_list_insert_here_)),
"Must have valid bounds or exactly one bulk insert or use list insertion"
);
}
Expand All @@ -411,6 +412,7 @@ struct ConstructParams {
| label_;
s.skip(list_inserts_);
s.skip(list_insert_here_);
s.skip(did_list_insert_here_);
s.skip(cons_fn_);
}

Expand All @@ -420,6 +422,7 @@ struct ConstructParams {
std::vector<IndexType> bulk_inserts_ = {};
std::vector<ListInsertType> list_inserts_ = {};
std::vector<ListInsertElmType> list_insert_here_ = {};
bool did_list_insert_here_ = false;
bool bulk_insert_bounds_ = false;
ConstructFnType cons_fn_ = nullptr;
bool dynamic_membership_ = false;
Expand Down
62 changes: 55 additions & 7 deletions tests/unit/collection/test_list_insert.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,13 +248,13 @@ TEST_F(TestListInsert, test_bounded_list_insert_here_3) {
num_work = 0;

auto const num_nodes = theContext()->getNumNodes();

auto const this_node = theContext()->getNode();
auto const range = Index1D(num_nodes * num_elms_per_node);

std::vector<std::tuple<vt::Index1D, std::unique_ptr<ListInsertTest>>> elms;

for (int i = 0; i < range.x(); i++) {
if (i % num_nodes == 0) {
if (i % num_nodes == this_node) {
Index1D ix{i};
elms.emplace_back(
std::make_tuple(ix, std::make_unique<ListInsertTest>())
Expand Down Expand Up @@ -282,11 +282,12 @@ TEST_F(TestListInsert, test_bounded_list_insert_here_no_default_constructor) {
num_work = 0;

auto const num_nodes = theContext()->getNumNodes();
auto const this_node = theContext()->getNode();
auto const range = Index1D(num_nodes * num_elms_per_node);

std::vector<std::tuple<vt::Index1D, std::unique_ptr<NonDefaultConstructibleStruct>>> elms;
for (int i = 0; i < range.x(); i++) {
if (i % num_nodes == 0) {
if (i % num_nodes == this_node) {
Index1D ix{i};
elms.emplace_back(
std::make_tuple(ix, std::make_unique<NonDefaultConstructibleStruct>(0))
Expand Down Expand Up @@ -314,13 +315,13 @@ TEST_F(TestListInsert, test_unbounded_list_insert_here_4) {
num_work = 0;

auto const num_nodes = theContext()->getNumNodes();

auto const this_node = theContext()->getNode();
auto const range = Index1D(num_nodes * num_elms_per_node);

std::vector<std::tuple<vt::Index1D, std::unique_ptr<ListInsertTest>>> elms;

for (int i = 0; i < range.x(); i++) {
if (i % num_nodes == 0) {
if (i % num_nodes == this_node) {
Index1D ix{i};
elms.emplace_back(
std::make_tuple(ix, std::make_unique<ListInsertTest>())
Expand Down Expand Up @@ -348,11 +349,12 @@ TEST_F(TestListInsert, test_unbounded_list_insert_here_no_default_constructor) {
num_work = 0;

auto const num_nodes = theContext()->getNumNodes();
auto const this_node = theContext()->getNode();
auto const range = Index1D(num_nodes * num_elms_per_node);

std::vector<std::tuple<vt::Index1D, std::unique_ptr<NonDefaultConstructibleStruct>>> elms;
for (int i = 0; i < range.x(); i++) {
if (i % num_nodes == 0) {
if (i % num_nodes == this_node) {
Index1D ix{i};
elms.emplace_back(
std::make_tuple(ix, std::make_unique<NonDefaultConstructibleStruct>(0))
Expand All @@ -375,6 +377,51 @@ TEST_F(TestListInsert, test_unbounded_list_insert_here_no_default_constructor) {
EXPECT_EQ(num_work, num_elms_per_node * num_nodes);
}

TEST_F(TestListInsert, test_unbounded_list_insert_here_no_default_constructor_empty_rank) {
num_inserted = 0;
num_work = 0;

auto const num_nodes = theContext()->getNumNodes();
auto const this_node = theContext()->getNode();
auto const range = Index1D((num_nodes - 1) * num_elms_per_node);

std::vector<std::tuple<vt::Index1D, std::unique_ptr<NonDefaultConstructibleStruct>>> elms;
for (int i = 0; i < range.x(); i++) {
if (i % (num_nodes - 1) == this_node) {
Index1D ix{i};
elms.emplace_back(
std::make_tuple(ix, std::make_unique<NonDefaultConstructibleStruct>(0))
);
}
}

auto proxy = vt::makeCollection<NonDefaultConstructibleStruct>(
"test_unbounded_list_insert_here_no_default_constructor_empty_rank"
)
.collective(true)
.listInsertHere(std::move(elms))
.template mapperObjGroupConstruct<MyMapper<Index1D>>()
.wait();

if (this_node < num_nodes - 1) {
EXPECT_EQ(num_inserted, num_elms_per_node);
} else {
EXPECT_EQ(num_inserted, 0);
}
num_inserted = 0;

runInEpochCollective([&]{
proxy.broadcast<WorkMsgNDC, &NonDefaultConstructibleStruct::work>();
});
if (this_node < num_nodes - 1) {
// all ranks broadcast to all other ranks
EXPECT_EQ(num_work, num_elms_per_node * num_nodes);
} else {
// but there is nothing on the final rank to do work
EXPECT_EQ(num_work, 0);
}
}

TEST_F(TestListInsert, test_bounded_bulk_insert_no_default_constructor) {
num_inserted = 0;
num_work = 0;
Expand Down Expand Up @@ -404,6 +451,7 @@ TEST_F(TestListInsert, test_bounded_mix_list_insert_no_default_constructor) {
num_work = 0;

auto const num_nodes = theContext()->getNumNodes();
auto const this_node = theContext()->getNode();
auto const range = Index1D(num_nodes * num_elms_per_node);

std::vector<Index1D> list_insert;
Expand All @@ -413,7 +461,7 @@ TEST_F(TestListInsert, test_bounded_mix_list_insert_no_default_constructor) {

std::vector<std::tuple<vt::Index1D, std::unique_ptr<NonDefaultConstructibleStruct>>> elms;
for (int i = range.x() / 2; i < range.x(); i++) {
if (i % num_nodes == 0) {
if (i % num_nodes == this_node) {
Index1D ix{i};
elms.emplace_back(
std::make_tuple(ix, std::make_unique<NonDefaultConstructibleStruct>(0))
Expand Down