From 100f8b648a71f0dd3bc6ab957a8d5db07c735e28 Mon Sep 17 00:00:00 2001 From: Shengting Cui Date: Mon, 5 Aug 2024 19:29:03 +0000 Subject: [PATCH 1/2] Remove duplicate outputs of realization config in MPI computation --- .../catchment/Formulation_Manager.hpp | 8 +++---- include/realizations/config/config.hpp | 4 ++-- include/realizations/config/formulation.hpp | 12 +++++++--- include/realizations/config/layer.hpp | 2 +- src/NGen.cpp | 2 +- test/core/multilayer/MultiLayerParserTest.cpp | 5 ++-- .../realizations/Formulation_Manager_Test.cpp | 23 ++++++++++++------- 7 files changed, 35 insertions(+), 21 deletions(-) diff --git a/include/realizations/catchment/Formulation_Manager.hpp b/include/realizations/catchment/Formulation_Manager.hpp index f587e1fe36..1ad2358b48 100644 --- a/include/realizations/catchment/Formulation_Manager.hpp +++ b/include/realizations/catchment/Formulation_Manager.hpp @@ -51,14 +51,14 @@ namespace realization { ~Formulation_Manager() = default; - void read(geojson::GeoJSON fabric, utils::StreamHandler output_stream) { + void read(geojson::GeoJSON fabric, utils::StreamHandler output_stream, int mpi_rank) { //TODO seperate the parsing of configuration options like time //and routing and other non feature specific tasks from this main function //which has to iterate the entire hydrofabric. auto possible_global_config = tree.get_child_optional("global"); if (possible_global_config) { - global_config = realization::config::Config(*possible_global_config); + global_config = realization::config::Config(*possible_global_config, mpi_rank); } auto possible_simulation_time = tree.get_child_optional("time"); @@ -91,7 +91,7 @@ namespace realization { for (std::pair layer_config : *layers_json_array) { - layer = config::Layer(layer_config.second); + layer = config::Layer(layer_config.second, mpi_rank); layer_desc = layer.get_descriptor(); // add the layer to storage @@ -152,7 +152,7 @@ namespace realization { #endif continue; } - realization::config::Config catchment_formulation(catchment_config.second); + realization::config::Config catchment_formulation(catchment_config.second, mpi_rank); if(!catchment_formulation.has_formulation()){ throw std::runtime_error("ERROR: No formulations defined for "+catchment_config.first+"."); diff --git a/include/realizations/config/config.hpp b/include/realizations/config/config.hpp index fb11dffa38..6f4fe9e178 100644 --- a/include/realizations/config/config.hpp +++ b/include/realizations/config/config.hpp @@ -26,7 +26,7 @@ namespace realization{ * * @param tree */ - Config(const boost::property_tree::ptree& tree){ + Config(const boost::property_tree::ptree& tree, int mpi_rank){ auto possible_forcing = tree.get_child_optional("forcing"); @@ -36,7 +36,7 @@ namespace realization{ //get first empty key under formulations (corresponds to first json array element) auto possible_formulation_tree = tree.get_child_optional("formulations.."); if(possible_formulation_tree){ - formulation = Formulation(*possible_formulation_tree); + formulation = Formulation(*possible_formulation_tree, mpi_rank); } } diff --git a/include/realizations/config/formulation.hpp b/include/realizations/config/formulation.hpp index 594a2900c1..24e6665c94 100644 --- a/include/realizations/config/formulation.hpp +++ b/include/realizations/config/formulation.hpp @@ -1,6 +1,7 @@ #ifndef NGEN_REALIZATION_CONFIG_FORMULATION_H #define NGEN_REALIZATION_CONFIG_FORMULATION_H +#include #include #include @@ -39,7 +40,7 @@ namespace realization{ * * @param tree property tree to build Formulation from */ - Formulation(const boost::property_tree::ptree& tree){ + Formulation(const boost::property_tree::ptree& tree, int mpi_rank){ type = tree.get("name"); for (std::pair setting : tree.get_child("params")) { //Construct the geoJSON PropertyMap from each key, value pair in "params" @@ -51,11 +52,16 @@ namespace realization{ if(type=="bmi_multi"){ for(auto& module : tree.get_child("params.modules")){ //Create the nested formulations in order of definition - nested.push_back(Formulation(module.second)); + nested.push_back(Formulation(module.second, mpi_rank)); } + #if NGEN_WITH_MPI + if (mpi_rank == 0) { geojson::JSONProperty::print_property(parameters.at("modules")); } - + #else + geojson::JSONProperty::print_property(parameters.at("modules")); + #endif + } } /** diff --git a/include/realizations/config/layer.hpp b/include/realizations/config/layer.hpp index 5a0546b537..e51c2d839c 100644 --- a/include/realizations/config/layer.hpp +++ b/include/realizations/config/layer.hpp @@ -27,7 +27,7 @@ namespace realization{ * * @param tree */ - Layer(const boost::property_tree::ptree& tree):formulation(tree){ + Layer(const boost::property_tree::ptree& tree, int mpi_rank):formulation(tree, mpi_rank){ std::vector missing_keys; auto name = tree.get_optional("name"); if(!name) missing_keys.push_back("name"); diff --git a/src/NGen.cpp b/src/NGen.cpp index 1c0cf2bf28..2f05e7b69e 100644 --- a/src/NGen.cpp +++ b/src/NGen.cpp @@ -394,7 +394,7 @@ int main(int argc, char *argv[]) { nexus_collection->update_ids("id"); std::cout<<"Initializing formulations" << std::endl; std::shared_ptr manager = std::make_shared(REALIZATION_CONFIG_PATH); - manager->read(catchment_collection, utils::getStdOut()); + manager->read(catchment_collection, utils::getStdOut(), mpi_rank); //TODO refactor manager->read so certain configs can be queried before the entire //realization collection is created diff --git a/test/core/multilayer/MultiLayerParserTest.cpp b/test/core/multilayer/MultiLayerParserTest.cpp index 77c3b679d8..6c526c1ed1 100644 --- a/test/core/multilayer/MultiLayerParserTest.cpp +++ b/test/core/multilayer/MultiLayerParserTest.cpp @@ -70,8 +70,9 @@ TEST_F(MultiLayerParserTest, TestInit0) TEST_F(MultiLayerParserTest, TestRead0) { + int mpi_rank = 0; manager = std::make_shared(realization_config_path.c_str()); - manager->read(catchment_collection, utils::getStdOut()); + manager->read(catchment_collection, utils::getStdOut(), mpi_rank); ASSERT_TRUE(true); -} \ No newline at end of file +} diff --git a/test/realizations/Formulation_Manager_Test.cpp b/test/realizations/Formulation_Manager_Test.cpp index ddfd3c1cdf..6325546366 100644 --- a/test/realizations/Formulation_Manager_Test.cpp +++ b/test/realizations/Formulation_Manager_Test.cpp @@ -713,7 +713,8 @@ TEST_F(Formulation_Manager_Test, basic_reading_1) { this->add_feature("cat-52"); this->add_feature("cat-67"); - manager.read(this->fabric, catchment_output); + int mpi_rank = 0; + manager.read(this->fabric, catchment_output, mpi_rank); ASSERT_EQ(manager.get_size(), 2); @@ -736,7 +737,8 @@ TEST_F(Formulation_Manager_Test, basic_reading_2) { this->add_feature("cat-52"); this->add_feature("cat-67"); - manager.read(this->fabric, catchment_output); + int mpi_rank = 0; + manager.read(this->fabric, catchment_output, mpi_rank); ASSERT_EQ(manager.get_size(), 2); @@ -757,7 +759,8 @@ TEST_F(Formulation_Manager_Test, basic_run_1) { this->add_feature("cat-52"); this->add_feature("cat-67"); - manager.read(this->fabric, catchment_output); + int mpi_rank = 0; + manager.read(this->fabric, catchment_output, mpi_rank); ASSERT_EQ(manager.get_size(), 2); @@ -791,7 +794,8 @@ TEST_F(Formulation_Manager_Test, basic_run_3) { realization::Formulation_Manager manager = realization::Formulation_Manager(stream); this->add_feature("cat-67"); - manager.read(this->fabric, catchment_output); + int mpi_rank = 0; + manager.read(this->fabric, catchment_output, mpi_rank); ASSERT_EQ(manager.get_size(), 1); ASSERT_TRUE(manager.contains("cat-67")); @@ -828,7 +832,8 @@ TEST_F(Formulation_Manager_Test, read_extra) { ASSERT_TRUE(manager.is_empty()); this->add_feature("cat-67"); - manager.read(this->fabric, catchment_output); + int mpi_rank = 0; + manager.read(this->fabric, catchment_output, mpi_rank); ASSERT_EQ(manager.get_size(), 1); ASSERT_TRUE(manager.contains("cat-67")); @@ -846,7 +851,8 @@ TEST_F(Formulation_Manager_Test, forcing_provider_specification) { this->add_feature("cat-67"); this->add_feature("cat-27115"); - manager.read(this->fabric, catchment_output); + int mpi_rank = 0; + manager.read(this->fabric, catchment_output, mpi_rank); ASSERT_EQ(manager.get_size(), 2); ASSERT_TRUE(manager.contains("cat-67")); @@ -942,7 +948,8 @@ TEST_F(Formulation_Manager_Test, read_external_attributes) { { "e", geojson::JSONProperty{"e", 2.71828 } } }); - manager.read(this->fabric, catchment_output); + int mpi_rank = 0; + manager.read(this->fabric, catchment_output, mpi_rank); ASSERT_EQ(manager.get_size(), 3); check_formulation_values(manager, "cat-67", { 1.70352, 10.0 }); @@ -960,7 +967,7 @@ TEST_F(Formulation_Manager_Test, read_external_attributes) { { "val", geojson::JSONProperty{"val", 7.41722 } } }); - manager.read(this->fabric, catchment_output); + manager.read(this->fabric, catchment_output, mpi_rank); check_formulation_values(manager, "cat-67", { 7.41722, 9231 }); } From af0506991af1ccb72cae20dbb75857a28761e61b Mon Sep 17 00:00:00 2001 From: Shengting Cui Date: Tue, 6 Aug 2024 20:49:29 +0000 Subject: [PATCH 2/2] Removing passing mpi_rank through functions --- .../catchment/Formulation_Manager.hpp | 8 +++---- include/realizations/config/config.hpp | 4 ++-- include/realizations/config/formulation.hpp | 20 ++++++++++------ include/realizations/config/layer.hpp | 2 +- src/NGen.cpp | 2 +- test/core/multilayer/MultiLayerParserTest.cpp | 5 ++-- .../realizations/Formulation_Manager_Test.cpp | 23 +++++++------------ 7 files changed, 31 insertions(+), 33 deletions(-) diff --git a/include/realizations/catchment/Formulation_Manager.hpp b/include/realizations/catchment/Formulation_Manager.hpp index 1ad2358b48..f587e1fe36 100644 --- a/include/realizations/catchment/Formulation_Manager.hpp +++ b/include/realizations/catchment/Formulation_Manager.hpp @@ -51,14 +51,14 @@ namespace realization { ~Formulation_Manager() = default; - void read(geojson::GeoJSON fabric, utils::StreamHandler output_stream, int mpi_rank) { + void read(geojson::GeoJSON fabric, utils::StreamHandler output_stream) { //TODO seperate the parsing of configuration options like time //and routing and other non feature specific tasks from this main function //which has to iterate the entire hydrofabric. auto possible_global_config = tree.get_child_optional("global"); if (possible_global_config) { - global_config = realization::config::Config(*possible_global_config, mpi_rank); + global_config = realization::config::Config(*possible_global_config); } auto possible_simulation_time = tree.get_child_optional("time"); @@ -91,7 +91,7 @@ namespace realization { for (std::pair layer_config : *layers_json_array) { - layer = config::Layer(layer_config.second, mpi_rank); + layer = config::Layer(layer_config.second); layer_desc = layer.get_descriptor(); // add the layer to storage @@ -152,7 +152,7 @@ namespace realization { #endif continue; } - realization::config::Config catchment_formulation(catchment_config.second, mpi_rank); + realization::config::Config catchment_formulation(catchment_config.second); if(!catchment_formulation.has_formulation()){ throw std::runtime_error("ERROR: No formulations defined for "+catchment_config.first+"."); diff --git a/include/realizations/config/config.hpp b/include/realizations/config/config.hpp index 6f4fe9e178..fb11dffa38 100644 --- a/include/realizations/config/config.hpp +++ b/include/realizations/config/config.hpp @@ -26,7 +26,7 @@ namespace realization{ * * @param tree */ - Config(const boost::property_tree::ptree& tree, int mpi_rank){ + Config(const boost::property_tree::ptree& tree){ auto possible_forcing = tree.get_child_optional("forcing"); @@ -36,7 +36,7 @@ namespace realization{ //get first empty key under formulations (corresponds to first json array element) auto possible_formulation_tree = tree.get_child_optional("formulations.."); if(possible_formulation_tree){ - formulation = Formulation(*possible_formulation_tree, mpi_rank); + formulation = Formulation(*possible_formulation_tree); } } diff --git a/include/realizations/config/formulation.hpp b/include/realizations/config/formulation.hpp index 24e6665c94..51b976be35 100644 --- a/include/realizations/config/formulation.hpp +++ b/include/realizations/config/formulation.hpp @@ -7,6 +7,10 @@ #include "JSONProperty.hpp" +#if NGEN_WITH_MPI +#include +#endif + namespace realization{ namespace config{ @@ -40,7 +44,7 @@ namespace realization{ * * @param tree property tree to build Formulation from */ - Formulation(const boost::property_tree::ptree& tree, int mpi_rank){ + Formulation(const boost::property_tree::ptree& tree){ type = tree.get("name"); for (std::pair setting : tree.get_child("params")) { //Construct the geoJSON PropertyMap from each key, value pair in "params" @@ -52,16 +56,18 @@ namespace realization{ if(type=="bmi_multi"){ for(auto& module : tree.get_child("params.modules")){ //Create the nested formulations in order of definition - nested.push_back(Formulation(module.second, mpi_rank)); + nested.push_back(Formulation(module.second)); } + // If running MPI job, output with only one processor #if NGEN_WITH_MPI - if (mpi_rank == 0) { - geojson::JSONProperty::print_property(parameters.at("modules")); - } - #else - geojson::JSONProperty::print_property(parameters.at("modules")); + int mpi_rank; + MPI_Comm_rank(MPI_COMM_WORLD, &mpi_rank); + if (mpi_rank == 0) #endif + { + geojson::JSONProperty::print_property(parameters.at("modules")); } + } } /** diff --git a/include/realizations/config/layer.hpp b/include/realizations/config/layer.hpp index e51c2d839c..5a0546b537 100644 --- a/include/realizations/config/layer.hpp +++ b/include/realizations/config/layer.hpp @@ -27,7 +27,7 @@ namespace realization{ * * @param tree */ - Layer(const boost::property_tree::ptree& tree, int mpi_rank):formulation(tree, mpi_rank){ + Layer(const boost::property_tree::ptree& tree):formulation(tree){ std::vector missing_keys; auto name = tree.get_optional("name"); if(!name) missing_keys.push_back("name"); diff --git a/src/NGen.cpp b/src/NGen.cpp index 2f05e7b69e..1c0cf2bf28 100644 --- a/src/NGen.cpp +++ b/src/NGen.cpp @@ -394,7 +394,7 @@ int main(int argc, char *argv[]) { nexus_collection->update_ids("id"); std::cout<<"Initializing formulations" << std::endl; std::shared_ptr manager = std::make_shared(REALIZATION_CONFIG_PATH); - manager->read(catchment_collection, utils::getStdOut(), mpi_rank); + manager->read(catchment_collection, utils::getStdOut()); //TODO refactor manager->read so certain configs can be queried before the entire //realization collection is created diff --git a/test/core/multilayer/MultiLayerParserTest.cpp b/test/core/multilayer/MultiLayerParserTest.cpp index 6c526c1ed1..77c3b679d8 100644 --- a/test/core/multilayer/MultiLayerParserTest.cpp +++ b/test/core/multilayer/MultiLayerParserTest.cpp @@ -70,9 +70,8 @@ TEST_F(MultiLayerParserTest, TestInit0) TEST_F(MultiLayerParserTest, TestRead0) { - int mpi_rank = 0; manager = std::make_shared(realization_config_path.c_str()); - manager->read(catchment_collection, utils::getStdOut(), mpi_rank); + manager->read(catchment_collection, utils::getStdOut()); ASSERT_TRUE(true); -} +} \ No newline at end of file diff --git a/test/realizations/Formulation_Manager_Test.cpp b/test/realizations/Formulation_Manager_Test.cpp index 6325546366..ddfd3c1cdf 100644 --- a/test/realizations/Formulation_Manager_Test.cpp +++ b/test/realizations/Formulation_Manager_Test.cpp @@ -713,8 +713,7 @@ TEST_F(Formulation_Manager_Test, basic_reading_1) { this->add_feature("cat-52"); this->add_feature("cat-67"); - int mpi_rank = 0; - manager.read(this->fabric, catchment_output, mpi_rank); + manager.read(this->fabric, catchment_output); ASSERT_EQ(manager.get_size(), 2); @@ -737,8 +736,7 @@ TEST_F(Formulation_Manager_Test, basic_reading_2) { this->add_feature("cat-52"); this->add_feature("cat-67"); - int mpi_rank = 0; - manager.read(this->fabric, catchment_output, mpi_rank); + manager.read(this->fabric, catchment_output); ASSERT_EQ(manager.get_size(), 2); @@ -759,8 +757,7 @@ TEST_F(Formulation_Manager_Test, basic_run_1) { this->add_feature("cat-52"); this->add_feature("cat-67"); - int mpi_rank = 0; - manager.read(this->fabric, catchment_output, mpi_rank); + manager.read(this->fabric, catchment_output); ASSERT_EQ(manager.get_size(), 2); @@ -794,8 +791,7 @@ TEST_F(Formulation_Manager_Test, basic_run_3) { realization::Formulation_Manager manager = realization::Formulation_Manager(stream); this->add_feature("cat-67"); - int mpi_rank = 0; - manager.read(this->fabric, catchment_output, mpi_rank); + manager.read(this->fabric, catchment_output); ASSERT_EQ(manager.get_size(), 1); ASSERT_TRUE(manager.contains("cat-67")); @@ -832,8 +828,7 @@ TEST_F(Formulation_Manager_Test, read_extra) { ASSERT_TRUE(manager.is_empty()); this->add_feature("cat-67"); - int mpi_rank = 0; - manager.read(this->fabric, catchment_output, mpi_rank); + manager.read(this->fabric, catchment_output); ASSERT_EQ(manager.get_size(), 1); ASSERT_TRUE(manager.contains("cat-67")); @@ -851,8 +846,7 @@ TEST_F(Formulation_Manager_Test, forcing_provider_specification) { this->add_feature("cat-67"); this->add_feature("cat-27115"); - int mpi_rank = 0; - manager.read(this->fabric, catchment_output, mpi_rank); + manager.read(this->fabric, catchment_output); ASSERT_EQ(manager.get_size(), 2); ASSERT_TRUE(manager.contains("cat-67")); @@ -948,8 +942,7 @@ TEST_F(Formulation_Manager_Test, read_external_attributes) { { "e", geojson::JSONProperty{"e", 2.71828 } } }); - int mpi_rank = 0; - manager.read(this->fabric, catchment_output, mpi_rank); + manager.read(this->fabric, catchment_output); ASSERT_EQ(manager.get_size(), 3); check_formulation_values(manager, "cat-67", { 1.70352, 10.0 }); @@ -967,7 +960,7 @@ TEST_F(Formulation_Manager_Test, read_external_attributes) { { "val", geojson::JSONProperty{"val", 7.41722 } } }); - manager.read(this->fabric, catchment_output, mpi_rank); + manager.read(this->fabric, catchment_output); check_formulation_values(manager, "cat-67", { 7.41722, 9231 }); }