From c849bdbaaf49f1979a27aaabe9b50e8f70a0927d Mon Sep 17 00:00:00 2001 From: Caleb Schilly Date: Thu, 5 Sep 2024 15:48:40 -0400 Subject: [PATCH 01/10] #2342: update schema to support seq_id in tasks field when id is not bit-encoded; add unit test --- scripts/JSON_data_files_validator.py | 7 +- scripts/LBDatafile_schema.py | 27 ++-- .../vrt/collection/balance/lb_data_holder.cc | 26 +++- tests/unit/lb/test_lb_data_holder.cc | 128 ++++++++++++++++++ 4 files changed, 169 insertions(+), 19 deletions(-) create mode 100644 tests/unit/lb/test_lb_data_holder.cc diff --git a/scripts/JSON_data_files_validator.py b/scripts/JSON_data_files_validator.py index 67abb5f688..05190ec508 100644 --- a/scripts/JSON_data_files_validator.py +++ b/scripts/JSON_data_files_validator.py @@ -435,11 +435,12 @@ def validate_comm_links(all_jsons): for data in all_jsons: if data["phases"][n].get("communications") is not None: comms = data["phases"][n]["communications"] - comm_ids.update({int(comm["from"]["id"]) for comm in comms}) - comm_ids.update({int(comm["to"]["id"]) for comm in comms}) + id_string = "id" if "id" in comms[0]["from"] else "seq_id" + comm_ids.update({int(comm["from"][id_string]) for comm in comms}) + comm_ids.update({int(comm["to"][id_string]) for comm in comms}) tasks = data["phases"][n]["tasks"] - task_ids.update({int(task["entity"]["id"]) for task in tasks}) + task_ids.update({int(task["entity"][id_string]) for task in tasks}) if not comm_ids.issubset(task_ids): logging.error( diff --git a/scripts/LBDatafile_schema.py b/scripts/LBDatafile_schema.py index bdbbeb84d2..743fff574e 100644 --- a/scripts/LBDatafile_schema.py +++ b/scripts/LBDatafile_schema.py @@ -1,5 +1,11 @@ from schema import And, Optional, Schema +def validate_id_and_seq_id(field): + """Ensure that either seq_id or id is provided.""" + if 'seq_id' not in field and 'id' not in field: + raise ValueError('Either id (bit-encoded) or seq_id must be provided.') + return field + LBDatafile_schema = Schema( { Optional('type'): And(str, "LBDatafile", error="'LBDatafile' must be chosen."), @@ -30,15 +36,16 @@ 'id': int, 'tasks': [ { - 'entity': { + 'entity': And({ Optional('collection_id'): int, 'home': int, - 'id': int, + Optional('id'): int, + Optional('seq_id'): int, Optional('index'): [int], 'type': str, 'migratable': bool, Optional('objgroup_id'): int - }, + }, validate_id_and_seq_id), 'node': int, 'resource': str, Optional('subphases'): [ @@ -55,25 +62,27 @@ Optional('communications'): [ { 'type': str, - 'to': { + 'to': And({ 'type': str, - 'id': int, + Optional('id'): int, + Optional('seq_id'): int, Optional('home'): int, Optional('collection_id'): int, Optional('migratable'): bool, Optional('index'): [int], Optional('objgroup_id'): int, - }, + }, validate_id_and_seq_id), 'messages': int, - 'from': { + 'from': And({ 'type': str, - 'id': int, + Optional('id'): int, + Optional('seq_id'): int, Optional('home'): int, Optional('collection_id'): int, Optional('migratable'): bool, Optional('index'): [int], Optional('objgroup_id'): int, - }, + }, validate_id_and_seq_id), 'bytes': float } ], diff --git a/src/vt/vrt/collection/balance/lb_data_holder.cc b/src/vt/vrt/collection/balance/lb_data_holder.cc index 333dc1babd..d47297ec9a 100644 --- a/src/vt/vrt/collection/balance/lb_data_holder.cc +++ b/src/vt/vrt/collection/balance/lb_data_holder.cc @@ -304,7 +304,14 @@ LBDataHolder::LBDataHolder(nlohmann::json const& j) vtAssertExpr(node.is_number()); if (etype == "object") { - auto object = task["entity"]["id"]; + nlohmann::json object; + bool bitpacked_id = false; + if (task["entity"].find("id") != task["entity"].end()) { + object = task["entity"]["id"]; + bitpacked_id = true; + } else { + object = task["entity"]["seq_id"]; + } vtAssertExpr(object.is_number()); auto elm = ElementIDStruct{object, node}; @@ -314,13 +321,18 @@ LBDataHolder::LBDataHolder(nlohmann::json const& j) task["entity"].find("index") != task["entity"].end() ) { using Field = uint64_t; - auto strippedObject = BitPackerType::getField< - vt::elm::eElmIDProxyBitsNonObjGroup::ID, - vt::elm::elm_id_num_bits, - Field - >(static_cast(object)); + Field object_id; + if (bitpacked_id) { + object_id = BitPackerType::getField< + vt::elm::eElmIDProxyBitsNonObjGroup::ID, + vt::elm::elm_id_num_bits, + Field + >(static_cast(object)); + } else { + object_id = static_cast(object); + } elm = elm::ElmIDBits::createCollectionImpl(migratable, - strippedObject, + object_id, home, node); auto cid = task["entity"]["collection_id"]; diff --git a/tests/unit/lb/test_lb_data_holder.cc b/tests/unit/lb/test_lb_data_holder.cc new file mode 100644 index 0000000000..8d330d6484 --- /dev/null +++ b/tests/unit/lb/test_lb_data_holder.cc @@ -0,0 +1,128 @@ +/* +//@HEADER +// ***************************************************************************** +// +// test_lb_data_holder.cc +// DARMA/vt => Virtual Transport +// +// Copyright 2019-2021 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 darma@sandia.gov +// +// ***************************************************************************** +//@HEADER +*/ + +#include "vt/elm/elm_id_bits.h" +#include +#include + +#include "test_harness.h" + +#include + +namespace vt { namespace tests { namespace unit { namespace lb { + +using TestLBDataHolder = TestHarness; + +nlohmann::json create_basic_json(std::string id_type, int id, + int home, int node, bool migratable) { + nlohmann::json j = { + {"metadata", { + {"rank", 0}, + {"type", "LBDatafile"} + }}, + {"phases", { + { + {"id", 0}, + {"tasks", { + { + {"entity", { + {"collection_id", 7}, + {"index", {0}}, + {"home", home}, + {id_type, id}, + {"migratable", migratable}, + {"type", "object"} + }}, + {"node", node}, + {"resource", "cpu"}, + {"time", 0.5}, + } + }} + } + }} + }; + + return j; +} + +void test_data_holder_elms(int seq_id, int home, int node, bool migratable) { + // Determine encoded ID + auto elm = elm::ElmIDBits::createCollectionImpl(migratable, seq_id, home, node); + auto encoded_id = elm.id; + + // Create DataHolder and get resulting object elm + auto simple_json_id = create_basic_json("id", encoded_id, home, node, migratable); + auto dh_id = vt::vrt::collection::balance::LBDataHolder(simple_json_id); + auto elm_id = dh_id.node_data_[0].begin()->first; + + // Create new DataHolder using "seq_id" and get elm + auto simple_json_seq = create_basic_json("seq_id", seq_id, home, node, migratable); + auto dh_seq = vt::vrt::collection::balance::LBDataHolder(simple_json_seq); + auto elm_seq = dh_seq.node_data_[0].begin()->first; + + // Assert that both elms are equal (have the same id) + EXPECT_EQ(elm_id, elm_seq); + EXPECT_EQ(elm_id, elm); +} + +TEST_F(TestLBDataHolder, test_lb_data_holder_no_comms_object_id) { + // Initialize + int argc = 0; + char** argv = nullptr; + MPI_Comm comm = MPI_COMM_WORLD; + vt::initialize(argc, argv, &comm); + + // Run a variety of test cases (seq_id, home, node, migratable) + test_data_holder_elms(0,0,0,false); + test_data_holder_elms(0,0,0,true); + test_data_holder_elms(0,0,2,false); + test_data_holder_elms(0,0,1,true); + test_data_holder_elms(1,1,0,false); + test_data_holder_elms(2,1,9,true); + test_data_holder_elms(3,0,1,false); + + // Finalize + // vt::finalize(); +} + +}}}} // end namespace vt::tests::unit::lb \ No newline at end of file From 8138deced7598eac62f7f20b6c64e210af22632e Mon Sep 17 00:00:00 2001 From: Caleb Schilly Date: Thu, 5 Sep 2024 17:34:44 -0400 Subject: [PATCH 02/10] #2342: wip: add logic for supporting or creating encoded ids in comm field --- scripts/JSON_data_files_validator.py | 8 +- .../vrt/collection/balance/lb_data_holder.cc | 102 ++++++++++++------ tests/unit/lb/test_lb_data_holder.cc | 11 +- 3 files changed, 73 insertions(+), 48 deletions(-) diff --git a/scripts/JSON_data_files_validator.py b/scripts/JSON_data_files_validator.py index 05190ec508..06e9f5b7ef 100644 --- a/scripts/JSON_data_files_validator.py +++ b/scripts/JSON_data_files_validator.py @@ -435,12 +435,12 @@ def validate_comm_links(all_jsons): for data in all_jsons: if data["phases"][n].get("communications") is not None: comms = data["phases"][n]["communications"] - id_string = "id" if "id" in comms[0]["from"] else "seq_id" - comm_ids.update({int(comm["from"][id_string]) for comm in comms}) - comm_ids.update({int(comm["to"][id_string]) for comm in comms}) + id_key = "id" if "id" in comms[0]["from"] else "seq_id" + comm_ids.update({int(comm["from"][id_key]) for comm in comms}) + comm_ids.update({int(comm["to"][id_key]) for comm in comms}) tasks = data["phases"][n]["tasks"] - task_ids.update({int(task["entity"][id_string]) for task in tasks}) + task_ids.update({int(task["entity"][id_key]) for task in tasks}) if not comm_ids.issubset(task_ids): logging.error( diff --git a/src/vt/vrt/collection/balance/lb_data_holder.cc b/src/vt/vrt/collection/balance/lb_data_holder.cc index d47297ec9a..9712697d13 100644 --- a/src/vt/vrt/collection/balance/lb_data_holder.cc +++ b/src/vt/vrt/collection/balance/lb_data_holder.cc @@ -41,14 +41,68 @@ //@HEADER */ -#include "vt/vrt/collection/balance/lb_data_holder.h" #include "vt/context/context.h" #include "vt/elm/elm_id_bits.h" +#include "vt/vrt/collection/balance/lb_data_holder.h" #include namespace vt { namespace vrt { namespace collection { namespace balance { +void get_object_from_json_field_( + const nlohmann::json& field, nlohmann::json& object, bool& bitpacked) { + if (field.find("id") != field.end()) { + object = field["id"]; + bitpacked = true; + } else { + object = field["seq_id"]; + bitpacked = false; + } +} + +ElementIDStruct get_elm_from_object_info_( + const nlohmann::json& object, bool bitpacked, bool migratable, + const nlohmann::json& home, const nlohmann::json& node) { + using Field = uint64_t; + + Field object_id; + if (bitpacked) { + object_id = BitPackerType::getField< + vt::elm::eElmIDProxyBitsNonObjGroup::ID, vt::elm::elm_id_num_bits, Field>( + static_cast(object)); + } else { + object_id = static_cast(object); + } + + return elm::ElmIDBits::createCollectionImpl( + migratable, object_id, home, node); +} + +ElementIDStruct get_elm_from_comm_object_(const nlohmann::json& field, bool collection) { + // Get the object's id and determine if it is bit-encoded + nlohmann::json object; + bool bitpacked_id; + get_object_from_json_field_(field, object, bitpacked_id); + vtAssertExpr(object.is_number()); + + // Somehow will this information + int home = 0; + int node = 0; + bool migratable = false; + + // Create elm with encoded data + ElementIDStruct elm; + if (collection) { + elm = + get_elm_from_object_info_(object, bitpacked_id, migratable, home, node); + } else { + elm = ElementIDStruct{object, theContext()->getNode()}; + } + + return elm; +} + + void LBDataHolder::outputEntity(nlohmann::json& j, ElementIDStruct const& id) const { j["type"] = "object"; j["id"] = id.id; @@ -278,8 +332,6 @@ std::unique_ptr LBDataHolder::toJson(PhaseType phase) const { LBDataHolder::LBDataHolder(nlohmann::json const& j) { - auto this_node = theContext()->getNode(); - // read metadata for skipped and identical phases readMetadata(j); @@ -314,27 +366,13 @@ LBDataHolder::LBDataHolder(nlohmann::json const& j) } vtAssertExpr(object.is_number()); - auto elm = ElementIDStruct{object, node}; + ElementIDStruct elm; if ( task["entity"].find("collection_id") != task["entity"].end() and - task["entity"].find("index") != task["entity"].end() - ) { - using Field = uint64_t; - Field object_id; - if (bitpacked_id) { - object_id = BitPackerType::getField< - vt::elm::eElmIDProxyBitsNonObjGroup::ID, - vt::elm::elm_id_num_bits, - Field - >(static_cast(object)); - } else { - object_id = static_cast(object); - } - elm = elm::ElmIDBits::createCollectionImpl(migratable, - object_id, - home, - node); + task["entity"].find("index") != task["entity"].end()) { + elm = get_elm_from_object_info_( + object, bitpacked_id, migratable, home, node); auto cid = task["entity"]["collection_id"]; auto idx = task["entity"]["index"]; if (cid.is_number() && idx.is_array()) { @@ -342,6 +380,8 @@ LBDataHolder::LBDataHolder(nlohmann::json const& j) auto proxy = static_cast(cid); this->node_idx_[elm] = std::make_tuple(proxy, arr); } + } else { + elm = ElementIDStruct{object, node}; } this->node_data_[id][elm].whole_phase_load = time; @@ -409,13 +449,11 @@ LBDataHolder::LBDataHolder(nlohmann::json const& j) vtAssertExpr(comm["from"]["type"] == "object"); vtAssertExpr(comm["to"]["type"] == "object"); - auto from_object = comm["from"]["id"]; - vtAssertExpr(from_object.is_number()); - auto from_elm = ElementIDStruct{from_object, this_node}; - - auto to_object = comm["to"]["id"]; - vtAssertExpr(to_object.is_number()); - auto to_elm = ElementIDStruct{to_object, this_node}; + // TODO: passing false here (and below) avoids encoding + // any information into the obj ids, which preserves + // the original behavior + auto from_elm = get_elm_from_comm_object_(comm["from"], false); + auto to_elm = get_elm_from_comm_object_(comm["to"], false); CommKey key( CommKey::CollectionTag{}, @@ -432,9 +470,7 @@ LBDataHolder::LBDataHolder(nlohmann::json const& j) auto from_node = comm["from"]["id"]; vtAssertExpr(from_node.is_number()); - auto to_object = comm["to"]["id"]; - vtAssertExpr(to_object.is_number()); - auto to_elm = ElementIDStruct{to_object, this_node}; + auto to_elm = get_elm_from_comm_object_(comm["to"], false); CommKey key( CommKey::NodeToCollectionTag{}, @@ -449,9 +485,7 @@ LBDataHolder::LBDataHolder(nlohmann::json const& j) vtAssertExpr(comm["from"]["type"] == "object"); vtAssertExpr(comm["to"]["type"] == "node"); - auto from_object = comm["from"]["id"]; - vtAssertExpr(from_object.is_number()); - auto from_elm = ElementIDStruct{from_object, this_node}; + auto from_elm = get_elm_from_comm_object_(comm["from"], false); auto to_node = comm["to"]["id"]; vtAssertExpr(to_node.is_number()); diff --git a/tests/unit/lb/test_lb_data_holder.cc b/tests/unit/lb/test_lb_data_holder.cc index 8d330d6484..cfcb3583af 100644 --- a/tests/unit/lb/test_lb_data_holder.cc +++ b/tests/unit/lb/test_lb_data_holder.cc @@ -106,12 +106,6 @@ void test_data_holder_elms(int seq_id, int home, int node, bool migratable) { } TEST_F(TestLBDataHolder, test_lb_data_holder_no_comms_object_id) { - // Initialize - int argc = 0; - char** argv = nullptr; - MPI_Comm comm = MPI_COMM_WORLD; - vt::initialize(argc, argv, &comm); - // Run a variety of test cases (seq_id, home, node, migratable) test_data_holder_elms(0,0,0,false); test_data_holder_elms(0,0,0,true); @@ -120,9 +114,6 @@ TEST_F(TestLBDataHolder, test_lb_data_holder_no_comms_object_id) { test_data_holder_elms(1,1,0,false); test_data_holder_elms(2,1,9,true); test_data_holder_elms(3,0,1,false); - - // Finalize - // vt::finalize(); } -}}}} // end namespace vt::tests::unit::lb \ No newline at end of file +}}}} // end namespace vt::tests::unit::lb From c36b530629a811d6f260510968d328444e02aa5f Mon Sep 17 00:00:00 2001 From: Caleb Schilly Date: Thu, 5 Sep 2024 17:40:17 -0400 Subject: [PATCH 03/10] #2342: reduce duplication and improve bool naming --- .../vrt/collection/balance/lb_data_holder.cc | 29 ++++++++----------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/src/vt/vrt/collection/balance/lb_data_holder.cc b/src/vt/vrt/collection/balance/lb_data_holder.cc index 9712697d13..efd4cca0e9 100644 --- a/src/vt/vrt/collection/balance/lb_data_holder.cc +++ b/src/vt/vrt/collection/balance/lb_data_holder.cc @@ -50,23 +50,23 @@ namespace vt { namespace vrt { namespace collection { namespace balance { void get_object_from_json_field_( - const nlohmann::json& field, nlohmann::json& object, bool& bitpacked) { + const nlohmann::json& field, nlohmann::json& object, bool& is_bitpacked) { if (field.find("id") != field.end()) { object = field["id"]; - bitpacked = true; + is_bitpacked = true; } else { object = field["seq_id"]; - bitpacked = false; + is_bitpacked = false; } } ElementIDStruct get_elm_from_object_info_( - const nlohmann::json& object, bool bitpacked, bool migratable, + const nlohmann::json& object, bool is_bitpacked, bool migratable, const nlohmann::json& home, const nlohmann::json& node) { using Field = uint64_t; Field object_id; - if (bitpacked) { + if (is_bitpacked) { object_id = BitPackerType::getField< vt::elm::eElmIDProxyBitsNonObjGroup::ID, vt::elm::elm_id_num_bits, Field>( static_cast(object)); @@ -81,11 +81,11 @@ ElementIDStruct get_elm_from_object_info_( ElementIDStruct get_elm_from_comm_object_(const nlohmann::json& field, bool collection) { // Get the object's id and determine if it is bit-encoded nlohmann::json object; - bool bitpacked_id; - get_object_from_json_field_(field, object, bitpacked_id); + bool is_bitpacked; + get_object_from_json_field_(field, object, is_bitpacked); vtAssertExpr(object.is_number()); - // Somehow will this information + // TODO: need to get this information (if it's relevant) int home = 0; int node = 0; bool migratable = false; @@ -94,7 +94,7 @@ ElementIDStruct get_elm_from_comm_object_(const nlohmann::json& field, bool coll ElementIDStruct elm; if (collection) { elm = - get_elm_from_object_info_(object, bitpacked_id, migratable, home, node); + get_elm_from_object_info_(object, is_bitpacked, migratable, home, node); } else { elm = ElementIDStruct{object, theContext()->getNode()}; } @@ -357,13 +357,8 @@ LBDataHolder::LBDataHolder(nlohmann::json const& j) if (etype == "object") { nlohmann::json object; - bool bitpacked_id = false; - if (task["entity"].find("id") != task["entity"].end()) { - object = task["entity"]["id"]; - bitpacked_id = true; - } else { - object = task["entity"]["seq_id"]; - } + bool is_bitpacked; + get_object_from_json_field_(task["entity"], object, is_bitpacked); vtAssertExpr(object.is_number()); ElementIDStruct elm; @@ -372,7 +367,7 @@ LBDataHolder::LBDataHolder(nlohmann::json const& j) task["entity"].find("collection_id") != task["entity"].end() and task["entity"].find("index") != task["entity"].end()) { elm = get_elm_from_object_info_( - object, bitpacked_id, migratable, home, node); + object, is_bitpacked, migratable, home, node); auto cid = task["entity"]["collection_id"]; auto idx = task["entity"]["index"]; if (cid.is_number() && idx.is_array()) { From 9386580c18c98c893fc5348ccb7bd1f1dc2c65db Mon Sep 17 00:00:00 2001 From: Caleb Schilly Date: Thu, 5 Sep 2024 17:46:42 -0400 Subject: [PATCH 04/10] #2342: fix scope error --- .../vrt/collection/balance/lb_data_holder.cc | 26 ++++++++++--------- tests/unit/lb/test_lb_data_holder.cc | 14 +++++----- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/src/vt/vrt/collection/balance/lb_data_holder.cc b/src/vt/vrt/collection/balance/lb_data_holder.cc index efd4cca0e9..cfebb9f4a5 100644 --- a/src/vt/vrt/collection/balance/lb_data_holder.cc +++ b/src/vt/vrt/collection/balance/lb_data_holder.cc @@ -61,7 +61,7 @@ void get_object_from_json_field_( } ElementIDStruct get_elm_from_object_info_( - const nlohmann::json& object, bool is_bitpacked, bool migratable, + const nlohmann::json& object, bool is_bitpacked, bool is_migratable, const nlohmann::json& home, const nlohmann::json& node) { using Field = uint64_t; @@ -75,26 +75,28 @@ ElementIDStruct get_elm_from_object_info_( } return elm::ElmIDBits::createCollectionImpl( - migratable, object_id, home, node); + is_migratable, object_id, home, node); } -ElementIDStruct get_elm_from_comm_object_(const nlohmann::json& field, bool collection) { +ElementIDStruct +get_elm_from_comm_object_(const nlohmann::json& field, bool collection) { // Get the object's id and determine if it is bit-encoded nlohmann::json object; bool is_bitpacked; get_object_from_json_field_(field, object, is_bitpacked); vtAssertExpr(object.is_number()); - // TODO: need to get this information (if it's relevant) - int home = 0; - int node = 0; - bool migratable = false; - // Create elm with encoded data ElementIDStruct elm; + if (collection) { - elm = - get_elm_from_object_info_(object, is_bitpacked, migratable, home, node); + // TODO: need to get actual object data + int home = 0; + int node = 0; + bool is_migratable = false; + + elm = get_elm_from_object_info_( + object, is_bitpacked, is_migratable, home, node); } else { elm = ElementIDStruct{object, theContext()->getNode()}; } @@ -350,7 +352,7 @@ LBDataHolder::LBDataHolder(nlohmann::json const& j) auto time = task["time"]; auto etype = task["entity"]["type"]; auto home = task["entity"]["home"]; - bool migratable = task["entity"]["migratable"]; + bool is_migratable = task["entity"]["migratable"]; vtAssertExpr(time.is_number()); vtAssertExpr(node.is_number()); @@ -367,7 +369,7 @@ LBDataHolder::LBDataHolder(nlohmann::json const& j) task["entity"].find("collection_id") != task["entity"].end() and task["entity"].find("index") != task["entity"].end()) { elm = get_elm_from_object_info_( - object, is_bitpacked, migratable, home, node); + object, is_bitpacked, is_migratable, home, node); auto cid = task["entity"]["collection_id"]; auto idx = task["entity"]["index"]; if (cid.is_number() && idx.is_array()) { diff --git a/tests/unit/lb/test_lb_data_holder.cc b/tests/unit/lb/test_lb_data_holder.cc index cfcb3583af..4494123ff4 100644 --- a/tests/unit/lb/test_lb_data_holder.cc +++ b/tests/unit/lb/test_lb_data_holder.cc @@ -54,7 +54,7 @@ namespace vt { namespace tests { namespace unit { namespace lb { using TestLBDataHolder = TestHarness; nlohmann::json create_basic_json(std::string id_type, int id, - int home, int node, bool migratable) { + int home, int node, bool is_migratable) { nlohmann::json j = { {"metadata", { {"rank", 0}, @@ -70,7 +70,7 @@ nlohmann::json create_basic_json(std::string id_type, int id, {"index", {0}}, {"home", home}, {id_type, id}, - {"migratable", migratable}, + {"migratable", is_migratable}, {"type", "object"} }}, {"node", node}, @@ -85,18 +85,18 @@ nlohmann::json create_basic_json(std::string id_type, int id, return j; } -void test_data_holder_elms(int seq_id, int home, int node, bool migratable) { +void test_data_holder_elms(int seq_id, int home, int node, bool is_migratable) { // Determine encoded ID - auto elm = elm::ElmIDBits::createCollectionImpl(migratable, seq_id, home, node); + auto elm = elm::ElmIDBits::createCollectionImpl(is_migratable, seq_id, home, node); auto encoded_id = elm.id; // Create DataHolder and get resulting object elm - auto simple_json_id = create_basic_json("id", encoded_id, home, node, migratable); + auto simple_json_id = create_basic_json("id", encoded_id, home, node, is_migratable); auto dh_id = vt::vrt::collection::balance::LBDataHolder(simple_json_id); auto elm_id = dh_id.node_data_[0].begin()->first; // Create new DataHolder using "seq_id" and get elm - auto simple_json_seq = create_basic_json("seq_id", seq_id, home, node, migratable); + auto simple_json_seq = create_basic_json("seq_id", seq_id, home, node, is_migratable); auto dh_seq = vt::vrt::collection::balance::LBDataHolder(simple_json_seq); auto elm_seq = dh_seq.node_data_[0].begin()->first; @@ -106,7 +106,7 @@ void test_data_holder_elms(int seq_id, int home, int node, bool migratable) { } TEST_F(TestLBDataHolder, test_lb_data_holder_no_comms_object_id) { - // Run a variety of test cases (seq_id, home, node, migratable) + // Run a variety of test cases (seq_id, home, node, is_migratable) test_data_holder_elms(0,0,0,false); test_data_holder_elms(0,0,0,true); test_data_holder_elms(0,0,2,false); From a9ec85f7e77055172f234af80c5cd6c3c717a016 Mon Sep 17 00:00:00 2001 From: Caleb Schilly Date: Fri, 6 Sep 2024 14:05:26 -0400 Subject: [PATCH 05/10] #2342: add communications to LBDataHolder test --- .../vrt/collection/balance/lb_data_holder.cc | 62 ++++--- tests/unit/lb/test_lb_data_holder.cc | 173 ++++++++++++------ 2 files changed, 148 insertions(+), 87 deletions(-) diff --git a/src/vt/vrt/collection/balance/lb_data_holder.cc b/src/vt/vrt/collection/balance/lb_data_holder.cc index cfebb9f4a5..9430d84ebb 100644 --- a/src/vt/vrt/collection/balance/lb_data_holder.cc +++ b/src/vt/vrt/collection/balance/lb_data_holder.cc @@ -50,7 +50,8 @@ namespace vt { namespace vrt { namespace collection { namespace balance { void get_object_from_json_field_( - const nlohmann::json& field, nlohmann::json& object, bool& is_bitpacked) { + const nlohmann::json& field, nlohmann::json& object, bool& is_bitpacked, + bool& is_collection) { if (field.find("id") != field.end()) { object = field["id"]; is_bitpacked = true; @@ -58,11 +59,16 @@ void get_object_from_json_field_( object = field["seq_id"]; is_bitpacked = false; } + if (field.find("collection_id") != field.end()) { + is_collection = true; + } else { + is_collection = false; + } } ElementIDStruct get_elm_from_object_info_( const nlohmann::json& object, bool is_bitpacked, bool is_migratable, - const nlohmann::json& home, const nlohmann::json& node) { + const nlohmann::json& home) { using Field = uint64_t; Field object_id; @@ -75,28 +81,25 @@ ElementIDStruct get_elm_from_object_info_( } return elm::ElmIDBits::createCollectionImpl( - is_migratable, object_id, home, node); + is_migratable, object_id, home, theContext()->getNode()); } ElementIDStruct -get_elm_from_comm_object_(const nlohmann::json& field, bool collection) { +get_elm_from_comm_object_(const nlohmann::json& field) { // Get the object's id and determine if it is bit-encoded nlohmann::json object; bool is_bitpacked; - get_object_from_json_field_(field, object, is_bitpacked); + bool is_collection; + get_object_from_json_field_(field, object, is_bitpacked, is_collection); vtAssertExpr(object.is_number()); // Create elm with encoded data ElementIDStruct elm; - - if (collection) { - // TODO: need to get actual object data - int home = 0; - int node = 0; - bool is_migratable = false; - + if (is_collection) { + int home = field["home"]; + bool is_migratable = field["migratable"]; elm = get_elm_from_object_info_( - object, is_bitpacked, is_migratable, home, node); + object, is_bitpacked, is_migratable, home); } else { elm = ElementIDStruct{object, theContext()->getNode()}; } @@ -104,7 +107,6 @@ get_elm_from_comm_object_(const nlohmann::json& field, bool collection) { return elm; } - void LBDataHolder::outputEntity(nlohmann::json& j, ElementIDStruct const& id) const { j["type"] = "object"; j["id"] = id.id; @@ -359,23 +361,23 @@ LBDataHolder::LBDataHolder(nlohmann::json const& j) if (etype == "object") { nlohmann::json object; - bool is_bitpacked; - get_object_from_json_field_(task["entity"], object, is_bitpacked); + bool is_bitpacked, is_collection; + get_object_from_json_field_(task["entity"], object, is_bitpacked, is_collection); vtAssertExpr(object.is_number()); + // Creating elm from `tasks` field ElementIDStruct elm; - - if ( - task["entity"].find("collection_id") != task["entity"].end() and - task["entity"].find("index") != task["entity"].end()) { + if (is_collection) { elm = get_elm_from_object_info_( - object, is_bitpacked, is_migratable, home, node); + object, is_bitpacked, is_migratable, home); auto cid = task["entity"]["collection_id"]; - auto idx = task["entity"]["index"]; - if (cid.is_number() && idx.is_array()) { - std::vector arr = idx; - auto proxy = static_cast(cid); - this->node_idx_[elm] = std::make_tuple(proxy, arr); + if (task["entity"].find("index") != task["entity"].end()) { + auto idx = task["entity"]["index"]; + if (cid.is_number() && idx.is_array()) { + std::vector arr = idx; + auto proxy = static_cast(cid); + this->node_idx_[elm] = std::make_tuple(proxy, arr); + } } } else { elm = ElementIDStruct{object, node}; @@ -449,8 +451,8 @@ LBDataHolder::LBDataHolder(nlohmann::json const& j) // TODO: passing false here (and below) avoids encoding // any information into the obj ids, which preserves // the original behavior - auto from_elm = get_elm_from_comm_object_(comm["from"], false); - auto to_elm = get_elm_from_comm_object_(comm["to"], false); + auto from_elm = get_elm_from_comm_object_(comm["from"]); + auto to_elm = get_elm_from_comm_object_(comm["to"]); CommKey key( CommKey::CollectionTag{}, @@ -467,7 +469,7 @@ LBDataHolder::LBDataHolder(nlohmann::json const& j) auto from_node = comm["from"]["id"]; vtAssertExpr(from_node.is_number()); - auto to_elm = get_elm_from_comm_object_(comm["to"], false); + auto to_elm = get_elm_from_comm_object_(comm["to"]); CommKey key( CommKey::NodeToCollectionTag{}, @@ -482,7 +484,7 @@ LBDataHolder::LBDataHolder(nlohmann::json const& j) vtAssertExpr(comm["from"]["type"] == "object"); vtAssertExpr(comm["to"]["type"] == "node"); - auto from_elm = get_elm_from_comm_object_(comm["from"], false); + auto from_elm = get_elm_from_comm_object_(comm["from"]); auto to_node = comm["to"]["id"]; vtAssertExpr(to_node.is_number()); diff --git a/tests/unit/lb/test_lb_data_holder.cc b/tests/unit/lb/test_lb_data_holder.cc index 4494123ff4..10542714a6 100644 --- a/tests/unit/lb/test_lb_data_holder.cc +++ b/tests/unit/lb/test_lb_data_holder.cc @@ -45,75 +45,134 @@ #include #include -#include "test_harness.h" +#include "test_parallel_harness.h" #include namespace vt { namespace tests { namespace unit { namespace lb { -using TestLBDataHolder = TestHarness; - -nlohmann::json create_basic_json(std::string id_type, int id, - int home, int node, bool is_migratable) { - nlohmann::json j = { - {"metadata", { - {"rank", 0}, - {"type", "LBDatafile"} - }}, - {"phases", { - { - {"id", 0}, - {"tasks", { - { - {"entity", { - {"collection_id", 7}, - {"index", {0}}, - {"home", home}, - {id_type, id}, - {"migratable", is_migratable}, - {"type", "object"} - }}, - {"node", node}, - {"resource", "cpu"}, - {"time", 0.5}, - } - }} - } - }} - }; - - return j; +using TestLBDataHolder = TestParallelHarness; +using LBDataHolder = vt::vrt::collection::balance::LBDataHolder; + +nlohmann::json create_entity_(std::string id_type, int id, int home, bool is_migratable) { + nlohmann::json entity = { + {id_type, id}, + {"type", "object"}, + {"collection_id", 7}, + {"index", {0}}, + {"home", home}, + {"migratable", is_migratable}, + }; + return entity; } -void test_data_holder_elms(int seq_id, int home, int node, bool is_migratable) { - // Determine encoded ID - auto elm = elm::ElmIDBits::createCollectionImpl(is_migratable, seq_id, home, node); - auto encoded_id = elm.id; +nlohmann::json create_json_( + std::string id_type, int id_1, int id_2, int home, int node, + bool is_migratable) { + + auto entity_1 = create_entity_(id_type, id_1, home, is_migratable); + auto entity_2 = create_entity_(id_type, id_2, home, is_migratable); + + // Generate JSON + nlohmann::json j = { + {"metadata", {{"rank", 0}, {"type", "LBDatafile"}}}, + {"phases", + {{{"communications", + {{{"bytes", 2.0}, + {"from", entity_1}, + {"messages", 1}, + {"to", entity_2}, + {"type", "SendRecv"}}}}, + {"id", 0}, + {"tasks", + { + { + {"entity", entity_1}, + {"node", node}, + {"resource", "cpu"}, + {"time", 0.5}, + }, + { + {"entity", entity_2}, + {"node", node}, + {"resource", "cpu"}, + {"time", 0.5}, + } + } + }}} + } + }; + + return j; +} + +void test_data_holder_elms(int seq_id_1, int home, int node, bool is_migratable) { + // Create a second seq_id + auto seq_id_2 = seq_id_1 + 1; + + // Determine encoded ID + auto elm_1 = + elm::ElmIDBits::createCollectionImpl(is_migratable, seq_id_1, home, node); + auto encoded_id_1 = elm_1.id; + + // Create second encoded ID + auto elm_2 = + elm::ElmIDBits::createCollectionImpl(is_migratable, seq_id_2, home, node); + auto encoded_id_2 = elm_2.id; + + // Create DataHolder and get resulting object elm + auto simple_json_id = create_json_( + "id", encoded_id_1, encoded_id_2, home, node, is_migratable); + auto dh_id = LBDataHolder(simple_json_id); + + // Create new DataHolder using "seq_id" and get elm + auto simple_json_seq = + create_json_("seq_id", seq_id_1, seq_id_2, home, node, is_migratable); + auto dh_seq = LBDataHolder(simple_json_seq); + + // Find both elms in each DataHolder + auto it_id_1 = dh_id.node_data_[0].find(elm_1); + auto it_seq_1 = dh_seq.node_data_[0].find(elm_1); + auto it_id_2 = dh_id.node_data_[0].find(elm_2); + auto it_seq_2 = dh_seq.node_data_[0].find(elm_2); + + // Assert that both elms exist + ASSERT_NE(it_id_1, dh_id.node_data_[0].end()); + ASSERT_NE(it_seq_1, dh_seq.node_data_[0].end()); + ASSERT_NE(it_id_2, dh_id.node_data_[0].end()); + ASSERT_NE(it_seq_2, dh_seq.node_data_[0].end()); + + // Compare the the elms in each DataHolder + EXPECT_EQ(it_id_1->first, elm_1); + EXPECT_EQ(it_seq_1->first, elm_1); + EXPECT_EQ(it_id_2->first, elm_2); + EXPECT_EQ(it_seq_2->first, elm_2); - // Create DataHolder and get resulting object elm - auto simple_json_id = create_basic_json("id", encoded_id, home, node, is_migratable); - auto dh_id = vt::vrt::collection::balance::LBDataHolder(simple_json_id); - auto elm_id = dh_id.node_data_[0].begin()->first; + // Check the communication data + auto comm_id = dh_id.node_comm_[0]; + auto comm_key_id = comm_id.begin()->first; + auto comm_seq = dh_seq.node_comm_[0]; + auto comm_key_seq = comm_seq.begin()->first; - // Create new DataHolder using "seq_id" and get elm - auto simple_json_seq = create_basic_json("seq_id", seq_id, home, node, is_migratable); - auto dh_seq = vt::vrt::collection::balance::LBDataHolder(simple_json_seq); - auto elm_seq = dh_seq.node_data_[0].begin()->first; + // Ensure that we get the same CommKey from both id types + EXPECT_EQ(comm_key_id, comm_key_seq); - // Assert that both elms are equal (have the same id) - EXPECT_EQ(elm_id, elm_seq); - EXPECT_EQ(elm_id, elm); + // Assert that both elms are present in the communication data + EXPECT_EQ(comm_key_id.fromObj(), elm_1); + EXPECT_EQ(comm_key_id.toObj(), elm_2); + EXPECT_EQ(comm_key_seq.fromObj(), elm_1); + EXPECT_EQ(comm_key_seq.toObj(), elm_2); } -TEST_F(TestLBDataHolder, test_lb_data_holder_no_comms_object_id) { - // Run a variety of test cases (seq_id, home, node, is_migratable) - test_data_holder_elms(0,0,0,false); - test_data_holder_elms(0,0,0,true); - test_data_holder_elms(0,0,2,false); - test_data_holder_elms(0,0,1,true); - test_data_holder_elms(1,1,0,false); - test_data_holder_elms(2,1,9,true); - test_data_holder_elms(3,0,1,false); +TEST_F(TestLBDataHolder, test_lb_data_holder_object_id) { + // Run a variety of test cases (seq_id, home, node, is_migratable) + test_data_holder_elms(0, 0, 0, false); + test_data_holder_elms(0, 0, 0, true); + test_data_holder_elms(0, 0, 2, false); + test_data_holder_elms(0, 0, 1, true); + test_data_holder_elms(1, 1, 0, false); + test_data_holder_elms(2, 1, 1, true); + test_data_holder_elms(3, 0, 1, false); } }}}} // end namespace vt::tests::unit::lb From dc4903b0128decb403f187ce0879fe51864f95fa Mon Sep 17 00:00:00 2001 From: Caleb Schilly Date: Fri, 6 Sep 2024 14:09:30 -0400 Subject: [PATCH 06/10] #2342: remove todo comment --- src/vt/vrt/collection/balance/lb_data_holder.cc | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/vt/vrt/collection/balance/lb_data_holder.cc b/src/vt/vrt/collection/balance/lb_data_holder.cc index 9430d84ebb..75e7a56aeb 100644 --- a/src/vt/vrt/collection/balance/lb_data_holder.cc +++ b/src/vt/vrt/collection/balance/lb_data_holder.cc @@ -448,9 +448,6 @@ LBDataHolder::LBDataHolder(nlohmann::json const& j) vtAssertExpr(comm["from"]["type"] == "object"); vtAssertExpr(comm["to"]["type"] == "object"); - // TODO: passing false here (and below) avoids encoding - // any information into the obj ids, which preserves - // the original behavior auto from_elm = get_elm_from_comm_object_(comm["from"]); auto to_elm = get_elm_from_comm_object_(comm["to"]); From 530ff588198897bc2dcc2534659d9d60f51b43d0 Mon Sep 17 00:00:00 2001 From: Caleb Schilly Date: Fri, 6 Sep 2024 14:17:54 -0400 Subject: [PATCH 07/10] #2342: resolve warnings --- tests/unit/lb/test_lb_data_holder.cc | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/unit/lb/test_lb_data_holder.cc b/tests/unit/lb/test_lb_data_holder.cc index 10542714a6..58971e277b 100644 --- a/tests/unit/lb/test_lb_data_holder.cc +++ b/tests/unit/lb/test_lb_data_holder.cc @@ -52,7 +52,6 @@ namespace vt { namespace tests { namespace unit { namespace lb { using TestLBDataHolder = TestParallelHarness; -using LBDataHolder = vt::vrt::collection::balance::LBDataHolder; nlohmann::json create_entity_(std::string id_type, int id, int home, bool is_migratable) { nlohmann::json entity = { @@ -123,12 +122,12 @@ void test_data_holder_elms(int seq_id_1, int home, int node, bool is_migratable) // Create DataHolder and get resulting object elm auto simple_json_id = create_json_( "id", encoded_id_1, encoded_id_2, home, node, is_migratable); - auto dh_id = LBDataHolder(simple_json_id); + auto dh_id = vt::vrt::collection::balance::LBDataHolder(simple_json_id); // Create new DataHolder using "seq_id" and get elm auto simple_json_seq = create_json_("seq_id", seq_id_1, seq_id_2, home, node, is_migratable); - auto dh_seq = LBDataHolder(simple_json_seq); + auto dh_seq = vt::vrt::collection::balance::LBDataHolder(simple_json_seq); // Find both elms in each DataHolder auto it_id_1 = dh_id.node_data_[0].find(elm_1); From 4741dc8e8988a3ee96afa669ec661eb42c69f62a Mon Sep 17 00:00:00 2001 From: Caleb Schilly Date: Fri, 6 Sep 2024 14:33:47 -0400 Subject: [PATCH 08/10] #2342: fix id_key declaration in validator script --- scripts/JSON_data_files_validator.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts/JSON_data_files_validator.py b/scripts/JSON_data_files_validator.py index 06e9f5b7ef..72a5c51c38 100644 --- a/scripts/JSON_data_files_validator.py +++ b/scripts/JSON_data_files_validator.py @@ -433,15 +433,15 @@ def validate_comm_links(all_jsons): task_ids = set() for data in all_jsons: + tasks = data["phases"][n]["tasks"] + id_key = "id" if "id" in tasks[0]["entity"] else "seq_id" + task_ids.update({int(task["entity"][id_key]) for task in tasks}) + if data["phases"][n].get("communications") is not None: comms = data["phases"][n]["communications"] - id_key = "id" if "id" in comms[0]["from"] else "seq_id" comm_ids.update({int(comm["from"][id_key]) for comm in comms}) comm_ids.update({int(comm["to"][id_key]) for comm in comms}) - tasks = data["phases"][n]["tasks"] - task_ids.update({int(task["entity"][id_key]) for task in tasks}) - if not comm_ids.issubset(task_ids): logging.error( f" Phase {n}: Task ids: {comm_ids - task_ids}. Tasks are " From 411ccdb60eb85cf27d9b1509ec657b305c634b89 Mon Sep 17 00:00:00 2001 From: Caleb Schilly Date: Mon, 9 Sep 2024 13:02:28 -0400 Subject: [PATCH 09/10] #2342: simplify logic by creating elms directly from encoded ids --- .../vrt/collection/balance/lb_data_holder.cc | 67 +++---- .../vrt/collection/balance/lb_data_holder.h | 25 +++ tests/unit/collection/test_lb_data_holder.cc | 112 +++++++++++ tests/unit/lb/test_lb_data_holder.cc | 177 ------------------ 4 files changed, 162 insertions(+), 219 deletions(-) delete mode 100644 tests/unit/lb/test_lb_data_holder.cc diff --git a/src/vt/vrt/collection/balance/lb_data_holder.cc b/src/vt/vrt/collection/balance/lb_data_holder.cc index 75e7a56aeb..701d3d27bb 100644 --- a/src/vt/vrt/collection/balance/lb_data_holder.cc +++ b/src/vt/vrt/collection/balance/lb_data_holder.cc @@ -49,8 +49,8 @@ namespace vt { namespace vrt { namespace collection { namespace balance { -void get_object_from_json_field_( - const nlohmann::json& field, nlohmann::json& object, bool& is_bitpacked, +void LBDataHolder::getObjectFromJsonField_( + nlohmann::json const& field, nlohmann::json& object, bool& is_bitpacked, bool& is_collection) { if (field.find("id") != field.end()) { object = field["id"]; @@ -59,6 +59,7 @@ void get_object_from_json_field_( object = field["seq_id"]; is_bitpacked = false; } + vtAssertExpr(object.is_number()); if (field.find("collection_id") != field.end()) { is_collection = true; } else { @@ -66,42 +67,23 @@ void get_object_from_json_field_( } } -ElementIDStruct get_elm_from_object_info_( - const nlohmann::json& object, bool is_bitpacked, bool is_migratable, - const nlohmann::json& home) { - using Field = uint64_t; - - Field object_id; - if (is_bitpacked) { - object_id = BitPackerType::getField< - vt::elm::eElmIDProxyBitsNonObjGroup::ID, vt::elm::elm_id_num_bits, Field>( - static_cast(object)); - } else { - object_id = static_cast(object); - } - - return elm::ElmIDBits::createCollectionImpl( - is_migratable, object_id, home, theContext()->getNode()); -} - ElementIDStruct -get_elm_from_comm_object_(const nlohmann::json& field) { +LBDataHolder::getElmFromCommObject_( + nlohmann::json const& field) const { // Get the object's id and determine if it is bit-encoded nlohmann::json object; - bool is_bitpacked; - bool is_collection; - get_object_from_json_field_(field, object, is_bitpacked, is_collection); - vtAssertExpr(object.is_number()); + bool is_bitpacked, is_collection; + getObjectFromJsonField_(field, object, is_bitpacked, is_collection); // Create elm with encoded data ElementIDStruct elm; - if (is_collection) { + if (is_collection and not is_bitpacked) { int home = field["home"]; bool is_migratable = field["migratable"]; - elm = get_elm_from_object_info_( - object, is_bitpacked, is_migratable, home); + elm = elm::ElmIDBits::createCollectionImpl( + is_migratable, static_cast(object), home, this_node_); } else { - elm = ElementIDStruct{object, theContext()->getNode()}; + elm = ElementIDStruct{object, this_node_}; } return elm; @@ -336,6 +318,8 @@ std::unique_ptr LBDataHolder::toJson(PhaseType phase) const { LBDataHolder::LBDataHolder(nlohmann::json const& j) { + this_node_ = theContext()->getNode(); + // read metadata for skipped and identical phases readMetadata(j); @@ -362,14 +346,16 @@ LBDataHolder::LBDataHolder(nlohmann::json const& j) if (etype == "object") { nlohmann::json object; bool is_bitpacked, is_collection; - get_object_from_json_field_(task["entity"], object, is_bitpacked, is_collection); - vtAssertExpr(object.is_number()); + getObjectFromJsonField_(task["entity"], object, is_bitpacked, is_collection); + + // Create elm + ElementIDStruct elm = is_collection and not is_bitpacked + ? elm::ElmIDBits::createCollectionImpl( + is_migratable, static_cast(object), home, this_node_) + : ElementIDStruct{object, this_node_}; + this->node_data_[id][elm].whole_phase_load = time; - // Creating elm from `tasks` field - ElementIDStruct elm; if (is_collection) { - elm = get_elm_from_object_info_( - object, is_bitpacked, is_migratable, home); auto cid = task["entity"]["collection_id"]; if (task["entity"].find("index") != task["entity"].end()) { auto idx = task["entity"]["index"]; @@ -379,11 +365,8 @@ LBDataHolder::LBDataHolder(nlohmann::json const& j) this->node_idx_[elm] = std::make_tuple(proxy, arr); } } - } else { - elm = ElementIDStruct{object, node}; } - this->node_data_[id][elm].whole_phase_load = time; if (task.find("subphases") != task.end()) { auto subphases = task["subphases"]; @@ -448,8 +431,8 @@ LBDataHolder::LBDataHolder(nlohmann::json const& j) vtAssertExpr(comm["from"]["type"] == "object"); vtAssertExpr(comm["to"]["type"] == "object"); - auto from_elm = get_elm_from_comm_object_(comm["from"]); - auto to_elm = get_elm_from_comm_object_(comm["to"]); + auto from_elm = getElmFromCommObject_(comm["from"]); + auto to_elm = getElmFromCommObject_(comm["to"]); CommKey key( CommKey::CollectionTag{}, @@ -466,7 +449,7 @@ LBDataHolder::LBDataHolder(nlohmann::json const& j) auto from_node = comm["from"]["id"]; vtAssertExpr(from_node.is_number()); - auto to_elm = get_elm_from_comm_object_(comm["to"]); + auto to_elm = getElmFromCommObject_(comm["to"]); CommKey key( CommKey::NodeToCollectionTag{}, @@ -481,7 +464,7 @@ LBDataHolder::LBDataHolder(nlohmann::json const& j) vtAssertExpr(comm["from"]["type"] == "object"); vtAssertExpr(comm["to"]["type"] == "node"); - auto from_elm = get_elm_from_comm_object_(comm["from"]); + auto from_elm = getElmFromCommObject_(comm["from"]); auto to_node = comm["to"]["id"]; vtAssertExpr(to_node.is_number()); diff --git a/src/vt/vrt/collection/balance/lb_data_holder.h b/src/vt/vrt/collection/balance/lb_data_holder.h index a3badad166..8bd33cb284 100644 --- a/src/vt/vrt/collection/balance/lb_data_holder.h +++ b/src/vt/vrt/collection/balance/lb_data_holder.h @@ -127,6 +127,29 @@ struct LBDataHolder { void addInitialTask(nlohmann::json& j, std::size_t n) const; + /** + * \brief Determine the object ID from the tasks or communication field of + * input JSON + * + * \param[in] field the json field containing an object ID + * \param[in] object empty json object to be populated with the object's ID + * \param[in] is_bitpacked empty bool to be populated with whether or not + * the ID is bit-encoded + * \param[in] is_collection empty bool to be populated with whether + * or not the object belongs to a collection + */ + static void getObjectFromJsonField_( + nlohmann::json const& field, nlohmann::json& object, + bool& is_bitpacked, bool& is_collection); + + /** + * \brief Create an ElementIDStruct for the communication object + * + * \param[in] field the communication field for the desired object + * e.g. communications["to"] or communications["from"] + */ + ElementIDStruct getElmFromCommObject_(nlohmann::json const& field) const; + /** * \brief Read the LB phase's metadata * @@ -135,6 +158,8 @@ struct LBDataHolder { void readMetadata(nlohmann::json const& j); public: + /// The current node + NodeType this_node_; /// Node attributes for the current rank ElmUserDataType rank_attributes_; /// Node timings for each local object diff --git a/tests/unit/collection/test_lb_data_holder.cc b/tests/unit/collection/test_lb_data_holder.cc index d13f51fc6b..03a0016d1d 100644 --- a/tests/unit/collection/test_lb_data_holder.cc +++ b/tests/unit/collection/test_lb_data_holder.cc @@ -47,6 +47,7 @@ #include "test_helpers.h" #include "test_collection_common.h" +#include "vt/elm/elm_id_bits.h" #include "vt/vrt/collection/manager.h" #include "vt/vrt/collection/balance/lb_data_holder.h" @@ -90,6 +91,104 @@ void addPhasesDataToJson(nlohmann::json& json, PhaseType amountOfPhasesToAdd, st json["phases"] = phases; } +nlohmann::json createEntity_(std::string id_type, int id, int home, bool is_migratable) { + nlohmann::json entity = { + {id_type, id}, + {"type", "object"}, + {"collection_id", 7}, + {"index", {0}}, + {"home", home}, + {"migratable", is_migratable}, + }; + return entity; +} + +nlohmann::json createJson_( + std::string id_type, int id_1, int id_2, int home, int node, + bool is_migratable) { + + auto entity_1 = createEntity_(id_type, id_1, home, is_migratable); + auto entity_2 = createEntity_(id_type, id_2, home, is_migratable); + + // Generate JSON + nlohmann::json j = { + {"metadata", {{"rank", 0}, {"type", "LBDatafile"}}}, + {"phases", + {{{"communications", + {{{"bytes", 2.0}, + {"from", entity_1}, + {"messages", 1}, + {"to", entity_2}, + {"type", "SendRecv"}}}}, + {"id", 0}, + {"tasks", + { + { + {"entity", entity_1}, + {"node", node}, + {"resource", "cpu"}, + {"time", 0.5}, + }, + { + {"entity", entity_2}, + {"node", node}, + {"resource", "cpu"}, + {"time", 0.5}, + } + } + }}} + } + }; + + return j; +} + +void testDataHolderElms(int seq_id_1, int home, int node, bool is_migratable) { + // Create a second seq_id + auto seq_id_2 = seq_id_1 + 1; + + // Determine encoded ID + auto elm_1 = + elm::ElmIDBits::createCollectionImpl(is_migratable, seq_id_1, home, node); + auto encoded_id_1 = elm_1.id; + + // Create second encoded ID + auto elm_2 = + elm::ElmIDBits::createCollectionImpl(is_migratable, seq_id_2, home, node); + auto encoded_id_2 = elm_2.id; + + // Create DataHolder and get resulting object elm + auto simple_json_id = createJson_( + "id", encoded_id_1, encoded_id_2, home, node, is_migratable); + auto dh_id = vt::vrt::collection::balance::LBDataHolder(simple_json_id); + + // Create new DataHolder using "seq_id" and get elm + auto simple_json_seq = + createJson_("seq_id", seq_id_1, seq_id_2, home, node, is_migratable); + auto dh_seq = vt::vrt::collection::balance::LBDataHolder(simple_json_seq); + + // Assert that both elms exist in both DataHolders + ASSERT_NE(dh_id.node_data_[0].find(elm_1), dh_id.node_data_[0].end()); + ASSERT_NE(dh_seq.node_data_[0].find(elm_1), dh_seq.node_data_[0].end()); + ASSERT_NE(dh_id.node_data_[0].find(elm_2), dh_id.node_data_[0].end()); + ASSERT_NE(dh_seq.node_data_[0].find(elm_2), dh_seq.node_data_[0].end()); + + // Check the communication data + auto comm_id = dh_id.node_comm_[0]; + auto comm_key_id = comm_id.begin()->first; + auto comm_seq = dh_seq.node_comm_[0]; + auto comm_key_seq = comm_seq.begin()->first; + + // Ensure that we get the same CommKey from both id types + EXPECT_EQ(comm_key_id, comm_key_seq); + + // Assert that both elms are present in the communication data + EXPECT_EQ(comm_key_id.fromObj(), elm_1); + EXPECT_EQ(comm_key_id.toObj(), elm_2); + EXPECT_EQ(comm_key_seq.fromObj(), elm_1); + EXPECT_EQ(comm_key_seq.toObj(), elm_2); +} + TEST_F(TestLBDataHolder, test_no_metadata) { using LBDataHolder = vt::vrt::collection::balance::LBDataHolder; @@ -302,6 +401,19 @@ TEST_F(TestLBDataHolder, test_default_time_format) { } } +TEST_F(TestLBDataHolder, test_lb_data_holder_object_id) { + // Run a variety of test cases (seq_id, home, node, is_migratable) + auto current_node = theContext()->getNode(); + testDataHolderElms(0, 0, current_node, false); + testDataHolderElms(0, 0, current_node, true); + testDataHolderElms(1, 0, current_node, false); + testDataHolderElms(1, 0, current_node, true); + testDataHolderElms(0, 1, current_node, false); + testDataHolderElms(0, 1, current_node, true); + testDataHolderElms(3, 1, current_node, false); + testDataHolderElms(2, 2, current_node, true); +} + }}}} // end namespace vt::tests::unit::lb #endif /*vt_check_enabled(lblite)*/ diff --git a/tests/unit/lb/test_lb_data_holder.cc b/tests/unit/lb/test_lb_data_holder.cc deleted file mode 100644 index 58971e277b..0000000000 --- a/tests/unit/lb/test_lb_data_holder.cc +++ /dev/null @@ -1,177 +0,0 @@ -/* -//@HEADER -// ***************************************************************************** -// -// test_lb_data_holder.cc -// DARMA/vt => Virtual Transport -// -// Copyright 2019-2021 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 darma@sandia.gov -// -// ***************************************************************************** -//@HEADER -*/ - -#include "vt/elm/elm_id_bits.h" -#include -#include - -#include "test_parallel_harness.h" - -#include - -namespace vt { namespace tests { namespace unit { namespace lb { - -using TestLBDataHolder = TestParallelHarness; - -nlohmann::json create_entity_(std::string id_type, int id, int home, bool is_migratable) { - nlohmann::json entity = { - {id_type, id}, - {"type", "object"}, - {"collection_id", 7}, - {"index", {0}}, - {"home", home}, - {"migratable", is_migratable}, - }; - return entity; -} - -nlohmann::json create_json_( - std::string id_type, int id_1, int id_2, int home, int node, - bool is_migratable) { - - auto entity_1 = create_entity_(id_type, id_1, home, is_migratable); - auto entity_2 = create_entity_(id_type, id_2, home, is_migratable); - - // Generate JSON - nlohmann::json j = { - {"metadata", {{"rank", 0}, {"type", "LBDatafile"}}}, - {"phases", - {{{"communications", - {{{"bytes", 2.0}, - {"from", entity_1}, - {"messages", 1}, - {"to", entity_2}, - {"type", "SendRecv"}}}}, - {"id", 0}, - {"tasks", - { - { - {"entity", entity_1}, - {"node", node}, - {"resource", "cpu"}, - {"time", 0.5}, - }, - { - {"entity", entity_2}, - {"node", node}, - {"resource", "cpu"}, - {"time", 0.5}, - } - } - }}} - } - }; - - return j; -} - -void test_data_holder_elms(int seq_id_1, int home, int node, bool is_migratable) { - // Create a second seq_id - auto seq_id_2 = seq_id_1 + 1; - - // Determine encoded ID - auto elm_1 = - elm::ElmIDBits::createCollectionImpl(is_migratable, seq_id_1, home, node); - auto encoded_id_1 = elm_1.id; - - // Create second encoded ID - auto elm_2 = - elm::ElmIDBits::createCollectionImpl(is_migratable, seq_id_2, home, node); - auto encoded_id_2 = elm_2.id; - - // Create DataHolder and get resulting object elm - auto simple_json_id = create_json_( - "id", encoded_id_1, encoded_id_2, home, node, is_migratable); - auto dh_id = vt::vrt::collection::balance::LBDataHolder(simple_json_id); - - // Create new DataHolder using "seq_id" and get elm - auto simple_json_seq = - create_json_("seq_id", seq_id_1, seq_id_2, home, node, is_migratable); - auto dh_seq = vt::vrt::collection::balance::LBDataHolder(simple_json_seq); - - // Find both elms in each DataHolder - auto it_id_1 = dh_id.node_data_[0].find(elm_1); - auto it_seq_1 = dh_seq.node_data_[0].find(elm_1); - auto it_id_2 = dh_id.node_data_[0].find(elm_2); - auto it_seq_2 = dh_seq.node_data_[0].find(elm_2); - - // Assert that both elms exist - ASSERT_NE(it_id_1, dh_id.node_data_[0].end()); - ASSERT_NE(it_seq_1, dh_seq.node_data_[0].end()); - ASSERT_NE(it_id_2, dh_id.node_data_[0].end()); - ASSERT_NE(it_seq_2, dh_seq.node_data_[0].end()); - - // Compare the the elms in each DataHolder - EXPECT_EQ(it_id_1->first, elm_1); - EXPECT_EQ(it_seq_1->first, elm_1); - EXPECT_EQ(it_id_2->first, elm_2); - EXPECT_EQ(it_seq_2->first, elm_2); - - // Check the communication data - auto comm_id = dh_id.node_comm_[0]; - auto comm_key_id = comm_id.begin()->first; - auto comm_seq = dh_seq.node_comm_[0]; - auto comm_key_seq = comm_seq.begin()->first; - - // Ensure that we get the same CommKey from both id types - EXPECT_EQ(comm_key_id, comm_key_seq); - - // Assert that both elms are present in the communication data - EXPECT_EQ(comm_key_id.fromObj(), elm_1); - EXPECT_EQ(comm_key_id.toObj(), elm_2); - EXPECT_EQ(comm_key_seq.fromObj(), elm_1); - EXPECT_EQ(comm_key_seq.toObj(), elm_2); -} - -TEST_F(TestLBDataHolder, test_lb_data_holder_object_id) { - // Run a variety of test cases (seq_id, home, node, is_migratable) - test_data_holder_elms(0, 0, 0, false); - test_data_holder_elms(0, 0, 0, true); - test_data_holder_elms(0, 0, 2, false); - test_data_holder_elms(0, 0, 1, true); - test_data_holder_elms(1, 1, 0, false); - test_data_holder_elms(2, 1, 1, true); - test_data_holder_elms(3, 0, 1, false); -} - -}}}} // end namespace vt::tests::unit::lb From 5381a4c446ab4d34ad26abb3636609ad67b32121 Mon Sep 17 00:00:00 2001 From: Caleb Schilly Date: Mon, 9 Sep 2024 13:29:39 -0400 Subject: [PATCH 10/10] #2342: initialize this_node_ with vt::uninitialized_destination --- src/vt/vrt/collection/balance/lb_data_holder.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vt/vrt/collection/balance/lb_data_holder.h b/src/vt/vrt/collection/balance/lb_data_holder.h index 8bd33cb284..fb2c9fce48 100644 --- a/src/vt/vrt/collection/balance/lb_data_holder.h +++ b/src/vt/vrt/collection/balance/lb_data_holder.h @@ -159,7 +159,7 @@ struct LBDataHolder { public: /// The current node - NodeType this_node_; + NodeType this_node_ = vt::uninitialized_destination; /// Node attributes for the current rank ElmUserDataType rank_attributes_; /// Node timings for each local object