From de5d6eb245f259319905e600d92de6f2c3836d31 Mon Sep 17 00:00:00 2001 From: Phil Miller Date: Mon, 8 Sep 2025 10:08:14 -0700 Subject: [PATCH 1/9] Drop unused Network construction --- src/partitionGenerator.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/partitionGenerator.cpp b/src/partitionGenerator.cpp index 652da5b564..693186dbbe 100644 --- a/src/partitionGenerator.cpp +++ b/src/partitionGenerator.cpp @@ -474,11 +474,8 @@ int main(int argc, char* argv[]) std::string link_key = "toid"; - Network catchment_network(catchment_collection, &link_key); //Assumes dendritic, can add check in network if needed. PartitionVSet catchment_part, nexus_part; - - //catchment_network.print_network(); //build the remote connections from network // read the nexus hydrofabric, reuse the catchments From 864e0d1e64daf664d33e556bd6458b6e1ef9401a Mon Sep 17 00:00:00 2001 From: Phil Miller Date: Wed, 10 Sep 2025 09:04:53 -0700 Subject: [PATCH 2/9] Print CMake root source directory in configuration summary --- CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 212a3a5d49..1af0a95bac 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -403,6 +403,7 @@ message(STATUS "---------------------------------------------------------------- ngen_multiline_message( "NGen version: ${ngen_VERSION}" "Build configuration summary:" +" Source directory: ${CMAKE_SOURCE_DIR}" " Generator: ${CMAKE_GENERATOR}" " Build type: ${CMAKE_BUILD_TYPE}" " System: ${CMAKE_SYSTEM_NAME}" From b0bb3b897d5b0b30880515bc7b19c01123e15d60 Mon Sep 17 00:00:00 2001 From: Phil Miller Date: Wed, 10 Sep 2025 09:38:34 -0700 Subject: [PATCH 3/9] GeoJSON: Add definitions for a 'Sentinel' feature type besides the ones in the standard --- include/geojson/FeatureVisitor.hpp | 2 ++ include/geojson/features/FeatureBase.hpp | 21 ++++++++------ include/geojson/features/Features.hpp | 3 +- include/geojson/features/SentinelFeature.hpp | 29 ++++++++++++++++++++ 4 files changed, 46 insertions(+), 9 deletions(-) create mode 100644 include/geojson/features/SentinelFeature.hpp diff --git a/include/geojson/FeatureVisitor.hpp b/include/geojson/FeatureVisitor.hpp index 0e85703026..a97ede9b55 100644 --- a/include/geojson/FeatureVisitor.hpp +++ b/include/geojson/FeatureVisitor.hpp @@ -9,6 +9,7 @@ namespace geojson { class MultiLineStringFeature; class MultiPolygonFeature; class CollectionFeature; + class SentinelFeature; /** * An abstract class used to operate on the children of FeatureBase @@ -27,6 +28,7 @@ namespace geojson { virtual void visit(MultiLineStringFeature *feature) = 0; virtual void visit(MultiPolygonFeature *feature) = 0; virtual void visit(CollectionFeature* feature) = 0; + virtual void visit(SentinelFeature *feature) = 0; virtual ~FeatureVisitor() = default; }; diff --git a/include/geojson/features/FeatureBase.hpp b/include/geojson/features/FeatureBase.hpp index e1769385b1..a11669808f 100644 --- a/include/geojson/features/FeatureBase.hpp +++ b/include/geojson/features/FeatureBase.hpp @@ -41,16 +41,21 @@ namespace geojson { /** * Describes a type of features + * + * These are numbered in accordance with the GeoPackage + * 'well-known binary' (WKB) feature types, other than the + * additional 'Sentinel' value defined here */ enum class FeatureType { - None, /*!< Represents an empty feature with no sort of geometry */ - Point, /*!< Represents a feature that contains a single Point geometry */ - LineString, /*!< Represents a feature that is represented by a series of interconnected points */ - Polygon, /*!< Represents a feature that is represented by a defined area */ - MultiPoint, /*!< Represents a feature that is represented by many points */ - MultiLineString, /*!< Represents a feature that is represented by multiple series of interconnected points */ - MultiPolygon, /*!< Represents a feature that is represented by multiple areas */ - GeometryCollection /*!< Represents a feature that contains a collection of different types of geometry */ + None = 0, /*!< Represents an empty feature with no sort of geometry */ + Point = 1, /*!< Represents a feature that contains a single Point geometry */ + LineString = 2, /*!< Represents a feature that is represented by a series of interconnected points */ + Polygon = 3, /*!< Represents a feature that is represented by a defined area */ + MultiPoint = 4, /*!< Represents a feature that is represented by many points */ + MultiLineString = 5, /*!< Represents a feature that is represented by multiple series of interconnected points */ + MultiPolygon = 6, /*!< Represents a feature that is represented by multiple areas */ + GeometryCollection = 7, /*!< Represents a feature that contains a collection of different types of geometry */ + Sentinel = 100 /*!< Represents a 'dummy' feature included for computational consistency */ }; /** diff --git a/include/geojson/features/Features.hpp b/include/geojson/features/Features.hpp index cb71830054..5eeb9ad5ba 100644 --- a/include/geojson/features/Features.hpp +++ b/include/geojson/features/Features.hpp @@ -9,5 +9,6 @@ #include "MultiLineStringFeature.hpp" #include "MultiPolygonFeature.hpp" #include "CollectionFeature.hpp" +#include "SentinelFeature.hpp" -#endif // GEOJSON_FEATURE_H \ No newline at end of file +#endif // GEOJSON_FEATURE_H diff --git a/include/geojson/features/SentinelFeature.hpp b/include/geojson/features/SentinelFeature.hpp new file mode 100644 index 0000000000..ea9362c254 --- /dev/null +++ b/include/geojson/features/SentinelFeature.hpp @@ -0,0 +1,29 @@ +#ifndef GEOJSON_SENTINEL_FEATURE_H +#define GEOJSON_SENTINEL_FEATURE_H + +#include "FeatureBase.hpp" +#include +#include + +#include +#include +#include +#include + +namespace geojson { + + class SentinelFeature : public FeatureBase { + public: + SentinelFeature( + std::string new_id + ) : FeatureBase(std::move(new_id)) { + this->type = geojson::FeatureType::Sentinel; + } + + void visit(FeatureVisitor& visitor) override { + visitor.visit(this); + } + }; +} + +#endif // GEOJSON_SENTINEL_FEATURE_H From 77b867e45a89d6eadf43c4183078d0e418a791bd Mon Sep 17 00:00:00 2001 From: Phil Miller Date: Wed, 10 Sep 2025 14:50:05 -0700 Subject: [PATCH 4/9] Add sentinel wb-SENTINEL-nex-NNN features downstream of otherwise-terminal nexuses --- src/partitionGenerator.cpp | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/partitionGenerator.cpp b/src/partitionGenerator.cpp index 693186dbbe..be325530a6 100644 --- a/src/partitionGenerator.cpp +++ b/src/partitionGenerator.cpp @@ -514,6 +514,30 @@ int main(int argc, char* argv[]) //Do this before linking features so that the alt ids can lookup the correct feature global_nexus_collection->update_ids("id"); global_nexus_collection->link_features_from_property(nullptr, &link_key); + + // Store these separately to avoid iterator invalidation from inserting them eagerly + std::vector> sentinels; + + for (auto& feature : *global_nexus_collection) + { + auto id = feature->get_id(); + auto type = id.substr(0,3); + if (hy_features::identifiers::isNexus(type)) { + if (feature->get_number_of_destination_features() == 0) { + std::string sentinel_id = "wb-TERMINAL_SENTINEL-" + feature->get_id(); + geojson::Feature sentinel_feature = std::make_shared(sentinel_id); + sentinels.push_back(sentinel_feature); + feature->add_destination_feature(sentinel_feature.get()); + LOG("Nexus " + feature->get_id() + " has no destination features; adding " + sentinel_id + " below it", LogLevel::INFO); + } + } + } + + for (auto& sentinel : sentinels) + { + global_nexus_collection->add_feature(sentinel); + } + // make a global network Network global_network(global_nexus_collection); From 23cdd4c3a1bba4d4d72219dd65cd1636349e14b1 Mon Sep 17 00:00:00 2001 From: Phil Miller Date: Wed, 10 Sep 2025 14:51:00 -0700 Subject: [PATCH 5/9] Remove validation of nexus partition boundaries arbitrarily at partition cut lines --- src/partitionGenerator.cpp | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/src/partitionGenerator.cpp b/src/partitionGenerator.cpp index be325530a6..f5f0fe55cf 100644 --- a/src/partitionGenerator.cpp +++ b/src/partitionGenerator.cpp @@ -134,8 +134,6 @@ void generate_partitions(network::Network& network, const int& num_partitions, c nexus_set.reserve(partition_size); std::string part_id, partition_str; - std::string up_nexus; - std::string down_nexus; for(const auto& catchment : network.filter("cat", network::SortOrder::TransposedDepthFirstPreorder)){ if (partition < remainder) partition_size = partition_size_plus1; @@ -164,15 +162,7 @@ void generate_partitions(network::Network& network, const int& num_partitions, c counter++; if(counter == partition_size) { - //partgen_ss<<"nexus "< destinations = network.get_destination_ids(catchment); - if(destinations.size() == 0){ - partgen_ss <<"Error: Catchment "< Date: Wed, 10 Sep 2025 14:59:44 -0700 Subject: [PATCH 6/9] Adjust partition sizes for added sentinel features --- src/partitionGenerator.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/partitionGenerator.cpp b/src/partitionGenerator.cpp index f5f0fe55cf..6612a49657 100644 --- a/src/partitionGenerator.cpp +++ b/src/partitionGenerator.cpp @@ -107,15 +107,16 @@ void write_remote_connections(const PartitionVSet& catchment_part, const Partiti * * @param network * @param num_partitions - * @param num_catchments - * @param catchment_part + * @param catchment_part - which catchments will be simulation on each partition + * @param nexus_part - which nexuses have contributing catchments on each partition */ -void generate_partitions(network::Network& network, const int& num_partitions, const int& num_catchments, PartitionVSet& catchment_part, - PartitionVSet& nexus_part) +void generate_partitions(network::Network& network, const int& num_partitions, PartitionVSet& catchment_part, PartitionVSet& nexus_part) { + auto catchments = network.filter("cat", network::SortOrder::TransposedDepthFirstPreorder); + int partition = 0; int counter = 0; - int total = num_catchments; + int total = size(catchments); int partition_size = total/num_partitions; int partition_size_norm = partition_size; int remainder; @@ -134,7 +135,7 @@ void generate_partitions(network::Network& network, const int& num_partitions, c nexus_set.reserve(partition_size); std::string part_id, partition_str; - for(const auto& catchment : network.filter("cat", network::SortOrder::TransposedDepthFirstPreorder)){ + for(const auto& catchment : catchments){ if (partition < remainder) partition_size = partition_size_plus1; else @@ -526,7 +527,7 @@ int main(int argc, char* argv[]) Network global_network(global_nexus_collection); //Generate the partitioning - generate_partitions(global_network, num_partitions, num_catchments, catchment_part, nexus_part); + generate_partitions(global_network, num_partitions, catchment_part, nexus_part); //global_network.print_network(); From 75edb8d4daf524bd5deafc2a10cfe41096c6a536 Mon Sep 17 00:00:00 2001 From: Phil Miller Date: Mon, 8 Sep 2025 10:08:35 -0700 Subject: [PATCH 7/9] Logging and cleanup --- src/partitionGenerator.cpp | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/partitionGenerator.cpp b/src/partitionGenerator.cpp index 6612a49657..5ecb36a711 100644 --- a/src/partitionGenerator.cpp +++ b/src/partitionGenerator.cpp @@ -149,9 +149,7 @@ void generate_partitions(network::Network& network, const int& num_partitions, P } if(nexus_set.size() == 0){ partgen_ss <<"Error: Catchment "< remote_connections_vec; + for (int i = 0; i < num_partitions; ++i) { + partgen_ss << "Partition " << i << " catchments: " << catchment_part[i].size() << "\n"; + for (auto& c : catchment_part[i]) + partgen_ss << c << std::endl; + partgen_ss << "nexuses " << nexus_part[i].size() << "\n"; + for (auto& n : nexus_part[i]) + partgen_ss << n << std::endl; + LOG(partgen_ss.str(), LogLevel::DEBUG); partgen_ss.str(""); + } + int total_remotes = 0; // loop over all partitions by partition id for (int ipart=0; ipart < catchment_part.size(); ++ipart) From f00ab8566f18f48de12542b9530458bfc64ab727 Mon Sep 17 00:00:00 2001 From: Phil Miller Date: Wed, 10 Sep 2025 16:12:08 -0700 Subject: [PATCH 8/9] Add missing support for added SentinelFeature in FeatureVisitor test --- test/geojson/FeatureCollection_Test.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/geojson/FeatureCollection_Test.cpp b/test/geojson/FeatureCollection_Test.cpp index 18684ac21c..27cfa77f63 100644 --- a/test/geojson/FeatureCollection_Test.cpp +++ b/test/geojson/FeatureCollection_Test.cpp @@ -55,6 +55,10 @@ class Visitor : public geojson::FeatureVisitor { this->types.push_back("CollectionFeature"); } + void visit(geojson::SentinelFeature *feature) override { + this->types.push_back("SentinelFeature"); + } + std::string get(int index) { if( index >= 0 && index < types.size() ) return this->types[index]; From 56f4376679cae0078f76c17d1136e128146256af Mon Sep 17 00:00:00 2001 From: Phil Miller Date: Thu, 11 Sep 2025 10:15:27 -0700 Subject: [PATCH 9/9] Comment about what's going on with sentinel flowpaths --- src/NGen.cpp | 7 +++++++ src/partitionGenerator.cpp | 19 ++++++++++++++++--- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/src/NGen.cpp b/src/NGen.cpp index 4aed4cda84..6a047e37e2 100644 --- a/src/NGen.cpp +++ b/src/NGen.cpp @@ -444,6 +444,13 @@ int main(int argc, char* argv[]) { // TODO: Instead of iterating through a collection of FeatureBase objects mapping to catchments, // we instead want to iterate through HY_Catchment objects geojson::GeoJSON catchment_collection; + // As part of the fix for NOAA-OWP/ngen#284 / NGWPC-6553, + // partitioning may insert sentinel flowpaths downstream of + // terminal nexuses. Those sentinels will not exist in the + // catchmentDataFile. Their listing in catchment_subset_ids works + // because the respective geoFOO::read() functions return the + // intersection of features in the file and the specified subset, + // rather than erroring on missing features. if (boost::algorithm::ends_with(catchmentDataFile, "gpkg")) { #if NGEN_WITH_SQLITE3 try { diff --git a/src/partitionGenerator.cpp b/src/partitionGenerator.cpp index 5ecb36a711..f8b48ee616 100644 --- a/src/partitionGenerator.cpp +++ b/src/partitionGenerator.cpp @@ -500,9 +500,23 @@ int main(int argc, char* argv[]) global_nexus_collection->update_ids("id"); global_nexus_collection->link_features_from_property(nullptr, &link_key); - // Store these separately to avoid iterator invalidation from inserting them eagerly + // ngen misbehaves in gathering and storing output when a terminal + // nexus is fed by multiple catchments partitioned to different + // processes. This was recorded as NOAA-OWP/ngen#284 and NGWPC-6553. + // + // Address that here by inserting sentinel flowpaths downstream of + // those nexuses. Those sentinels will be assigned to specific + // processes, which will then properly receive all of the flow for + // the nexus in question. + // + // These features will not exist in the hydrofabric + // GeoJSON/GeoPackage. As implemented, that means that they will + // simply be ignored when ngen figures out what features to + // simulate on each process. + // + // Store the sentinels separately to avoid iterator invalidation + // from inserting them eagerly std::vector> sentinels; - for (auto& feature : *global_nexus_collection) { auto id = feature->get_id(); @@ -517,7 +531,6 @@ int main(int argc, char* argv[]) } } } - for (auto& sentinel : sentinels) { global_nexus_collection->add_feature(sentinel);