Reject duplicate PhasorDynamics IDs and report unknown signal IDs

CHANGELOG.md

@@ -1,6 +1,6 @@
 # Release Changelog
 
-## v.03
+## v0.3
 
 - Added `output_layout` solver option for long monitor output and a PostgreSQL example.
 - Restricted monitored bus names and device ids to letters, digits, spaces, `_`, `.` and `-`.
@@ -24,6 +24,9 @@
 - Fixed ESDC1A rate feedback to use the limited field-voltage rate.
 - Fixed REGCA to hold the terminal voltage at `Vhvmax`, as PowerWorld does; `Khv` is accepted but unused.
 - Added the FourBusLimits validation case, which drives the REPCA, REECB, REGCA, and ESDC1A limits.
+- Added a `TwoArea` integration test comparing flat and hierarchical PhasorDynamics systems.
+- Added rate feedback (`Kf`, `Tf`) to the IEEET1 exciters in the `TwoArea` case, whose generators lost synchronism without a fault.
+- Verify bus numbers, signal IDs, device IDs per class, and subsystem IDs in PhasorDynamics inputs.
 
 ## v0.2
 

GridKit/Model/PhasorDynamics/Component.hpp

@@ -608,8 +608,8 @@ namespace GridKit
       /// Global indices of attached external signals
       std::vector<IdxT> ws_indices_;
 
-      RealT time_;
-      RealT alpha_;
+      RealT time_{0};
+      RealT alpha_{0};
 
       RealT freq_system_base_{60.0};
       RealT va_system_base_{100.0e6};

GridKit/Model/PhasorDynamics/ComponentData.hpp

@@ -73,10 +73,16 @@ namespace GridKit
       /// Disambiguation string for this device
       std::string disambiguation_string;
 
-      /// Device class and disambiguation string (e.g. "Genrou_1"), for component lookups
+      /// Device class and disambiguation string (e.g. "Genrou_1"), for component lookups.
+      /// Empty for data built in code with neither, which is not looked up.
       std::string label() const
       {
-        return device_class + "_" + disambiguation_string;
+        std::string device_label;
+        if (!device_class.empty() || !disambiguation_string.empty())
+        {
+          device_label = device_class + "_" + disambiguation_string;
+        }
+        return device_label;
       }
     };
   } // namespace PhasorDynamics

GridKit/Model/PhasorDynamics/INPUT_FORMAT.md

@@ -170,7 +170,7 @@ Each subsystem also requires:
 
 Key | Description
 -- | --
-`id` | Subsystem ID (string), unique across the whole hierarchy
+`id` | Subsystem ID (non-empty string), unique across the whole hierarchy
 `level` | Hierarchy level (integer): the root system is level 0 (implied, not given in the file), its subsystems level 1, their subsystems level 2, and so on
 
 The `level` must match the depth of the subsystem. It is redundant in this
@@ -188,10 +188,6 @@ The input must already be partitioned:
 -   Devices may only use signals of their own system.
 -   Bus numbers and signal IDs must be unique across the whole hierarchy.
 
-Not all of these are checked yet: duplicate subsystem IDs, duplicate signal
-IDs across systems, and bus numbers repeated in sibling subsystems are not
-detected.
-
 ## Example File for a 2-Bus System
 
 ```json

GridKit/Model/PhasorDynamics/SignalNode/SignalNodeSet.hpp

@@ -7,17 +7,21 @@
 #pragma once
 
 #include <cassert>
+#include <stdexcept>
 #include <string>
 #include <unordered_map>
 #include <vector>
 
 #include <GridKit/Model/PhasorDynamics/SignalNode/SignalNode.hpp>
 #include <GridKit/Model/PhasorDynamics/SignalNode/SignalNodeData.hpp>
+#include <GridKit/Utilities/Logger/Logger.hpp>
 
 namespace GridKit
 {
   namespace PhasorDynamics
   {
+    using Log = ::GridKit::Utilities::Logger;
+
     /**
      * @brief Manage a collection of signal nodes specified for a system model
      *
@@ -59,9 +63,17 @@ namespace GridKit
 
       /**
        * @brief Access a SignalNode using its id
+       *
+       * @throws std::runtime_error if no node has this ID
        */
       SignalNodeT* operator[](IdxT signal_id)
       {
+        if (!signal_node_id_map_.contains(signal_id))
+        {
+          Log::error() << "Unknown signal ID " << signal_id << "\n";
+          throw std::runtime_error("SignalNodeSet lookup failed");
+        }
+
         auto* node = signal_node_id_map_.at(signal_id);
         assert(node->signalId() == signal_id);
         return node;
@@ -72,8 +84,13 @@ namespace GridKit
        */
       SignalNodeT* find(IdxT signal_id) const
       {
-        const auto it = signal_node_id_map_.find(signal_id);
-        return it == signal_node_id_map_.end() ? nullptr : it->second;
+        SignalNodeT* node = nullptr;
+        const auto   it   = signal_node_id_map_.find(signal_id);
+        if (it != signal_node_id_map_.end())
+        {
+          node = it->second;
+        }
+        return node;
       }
 
       /**
@@ -84,6 +101,14 @@ namespace GridKit
         return signal_node_name_map_.at(signal_name);
       }
 
+      /**
+       * @brief Signal nodes in the order they were added
+       */
+      const std::vector<SignalNodeT*>& nodes() const
+      {
+        return signal_nodes_;
+      }
+
     private:
       /// Vector of nodes
       std::vector<SignalNodeT*>                     signal_nodes_;

GridKit/Model/PhasorDynamics/SystemModel.hpp

@@ -125,6 +125,8 @@ namespace GridKit
       ComponentT*  getComponent(const std::string& label);
       BranchT*     getBranch(const std::string& id);
 
+      IdxT               numBuses() const;
+      IdxT               getBusId(IdxT gridkit_bus_id) const;
       IdxT               numComponents() const;
       const std::string& getComponentLabel(IdxT gridkit_component_id) const;
       IdxT               numSubsystems() const;
@@ -137,6 +139,7 @@ namespace GridKit
       BusT*        findBus(IdxT bus_id);
       SignalNodeT* findSignalNode(IdxT signal_id);
       ComponentT*  findComponent(const std::string& label);
+      void         checkUniqueIds(Model::ConfigurationChecks& checks) const;
       IdxT         computeSize();
       void         bindChildren(IdxT global_offset);
       void         addMonitorsTo(MonitorT& monitor) const;
@@ -157,9 +160,10 @@ namespace GridKit
       std::map<IdxT, BusT*> external_buses_;         ///< Parent's buses by bus_id, not owned
       std::vector<BusT*>    ordered_external_buses_; ///< External buses in construction order, not owned
 
-      IdxT offset_{0};           ///< Global index of this system's first variable
-      bool is_subsystem_{false}; ///< Allocated and bound by a parent system
-      bool owns_components_{false};
+      std::string id_;                  ///< Subsystem ID from the input data, empty if none
+      IdxT        offset_{0};           ///< Global index of this system's first variable
+      bool        is_subsystem_{false}; ///< Allocated and bound by a parent system
+      bool        owns_components_{false};
 
       /// Variable monitor
       std::unique_ptr<MonitorT> monitor_;

GridKit/Model/PhasorDynamics/SystemModelDataJSONParser.hpp

@@ -237,9 +237,10 @@ namespace GridKit
           subsystem.va_base   = sm.va_base;
           subsystem.level     = sm.level + 1;
 
-          if (!raw_subsystem.contains("id") || !raw_subsystem.at("id").is_string())
+          if (!raw_subsystem.contains("id") || !raw_subsystem.at("id").is_string()
+              || raw_subsystem.at("id").template get<std::string>().empty())
           {
-            Log::error() << "\n\tEach subsystem requires a string \"id\".\n";
+            Log::error() << "\n\tEach subsystem requires a non-empty string \"id\".\n";
             throw std::runtime_error("JSON parser failed");
           }
           raw_subsystem.at("id").get_to(subsystem.id);

GridKit/Model/PhasorDynamics/SystemModelImpl.hpp

@@ -1,6 +1,7 @@
 #include <cassert>
 #include <format>
 #include <iostream>
+#include <set>
 
 #include <GridKit/Definitions.hpp>
 #include <GridKit/Model/PhasorDynamics/Bus/BusFactory.hpp>
@@ -63,6 +64,7 @@ namespace GridKit
       using namespace Converter;
 
       owns_components_ = true;
+      id_              = data.id;
       monitor_         = std::make_unique<MonitorT>(time_);
 
       // Store parsed system bases before constructing data-driven components.
@@ -81,16 +83,13 @@ namespace GridKit
         addBus(bus);
       }
 
+      // A repeated ID keeps the first bus. verify() reports the duplicate.
       for (auto* bus : external_buses)
       {
-        if (gridkit_bus_indices_.contains(bus->busID())
-            || !external_buses_.emplace(bus->busID(), bus).second)
+        if (external_buses_.emplace(bus->busID(), bus).second)
         {
-          Log::error() << "External bus " << bus->busID()
-                       << " is already in the subsystem\n";
-          throw std::runtime_error("SystemModel construction failed");
+          ordered_external_buses_.push_back(bus);
         }
-        ordered_external_buses_.push_back(bus);
       }
 
       // Add REGCA converters
@@ -601,7 +600,7 @@ namespace GridKit
     }
 
     /**
-     * @brief Verify all components are configured correctly
+     * @brief Verify all components are configured correctly and IDs are unique
      *
      * Collects every problem reported by the components, each prefixed with
      * the component ID. passed() is true when all is well.
@@ -611,6 +610,12 @@ namespace GridKit
     {
       Model::ConfigurationChecks checks;
 
+      // IDs are unique across a hierarchy, so only its top system checks them.
+      if (!is_subsystem_)
+      {
+        checkUniqueIds(checks);
+      }
+
       // Verify components
       for (const auto& component : components_)
       {
@@ -971,6 +976,8 @@ namespace GridKit
      *
      * @pre The subsystem is not allocated and not part of another system, and
      * has the same system bases. This system has no devices.
+     *
+     * @note Repeated IDs are reported by verify().
      */
     template <typename scalar_type, typename index_type>
     void SystemModel<scalar_type, index_type>::addSubsystem(SystemModel* subsystem)
@@ -1000,6 +1007,59 @@ namespace GridKit
       allocated_ = false;
     }
 
+    /**
+     * @brief Report every bus, signal, component or subsystem ID that repeats
+     * in this system's tree
+     *
+     * One tree insert per ID, so the cost is N log N in the number of IDs.
+     */
+    template <typename scalar_type, typename index_type>
+    void SystemModel<scalar_type, index_type>::checkUniqueIds(Model::ConfigurationChecks& checks) const
+    {
+      std::set<IdxT>        bus_ids;
+      std::set<IdxT>        signal_ids;
+      std::set<std::string> labels;
+      std::set<std::string> subsystem_ids;
+
+      std::vector<const SystemModel*> systems{this};
+      for (std::size_t entry = 0; entry < systems.size(); ++entry)
+      {
+        const SystemModel* system = systems[entry];
+
+        if (!system->id_.empty() && !subsystem_ids.insert(system->id_).second)
+        {
+          checks.fail(std::format("Duplicate subsystem ID {}", system->id_));
+        }
+
+        // TODO: buses built in code share INVALID_INDEX, so they are not checked.
+        for (const auto* bus : system->buses_)
+        {
+          if (bus->busID() != INVALID_INDEX<IdxT> && !bus_ids.insert(bus->busID()).second)
+          {
+            checks.fail(std::format("Duplicate bus ID {}", bus->busID()));
+          }
+        }
+
+        for (const auto* node : system->signal_nodes_.nodes())
+        {
+          if (!signal_ids.insert(node->signalId()).second)
+          {
+            checks.fail(std::format("Duplicate signal ID {}", node->signalId()));
+          }
+        }
+
+        for (const auto& label : system->component_labels_)
+        {
+          if (!label.empty() && !labels.insert(label).second)
+          {
+            checks.fail(std::format("Duplicate component {}", label));
+          }
+        }
+
+        systems.insert(systems.end(), system->subsystems_.begin(), system->subsystems_.end());
+      }
+    }
+
     /**
      * @brief Set system bases and propagate them to existing components.
      *
@@ -1068,6 +1128,27 @@ namespace GridKit
       return bus;
     }
 
+    /**
+     * @brief Return the number of this system's own buses, without those of
+     * its subsystems
+     */
+    template <typename scalar_type, typename index_type>
+    typename SystemModel<scalar_type, index_type>::IdxT
+    SystemModel<scalar_type, index_type>::numBuses() const
+    {
+      return static_cast<IdxT>(buses_.size());
+    }
+
+    /**
+     * @brief Return the bus_id of one of this system's own buses
+     */
+    template <typename scalar_type, typename index_type>
+    typename SystemModel<scalar_type, index_type>::IdxT
+    SystemModel<scalar_type, index_type>::getBusId(IdxT gridkit_bus_id) const
+    {
+      return buses_.at(gridkit_bus_id)->busID();
+    }
+
     template <typename scalar_type, typename index_type>
     typename SystemModel<scalar_type, index_type>::IdxT
     SystemModel<scalar_type, index_type>::numComponents() const

cases/PhasorDynamics/TwoArea/README.md

@@ -10,6 +10,8 @@ Figure 1: One-line diagram of the TwoArea case.[^1]
 
 This case is derived from the Kundur two-area system and includes exciters for use in GridKit.
 
+The exciters use the rate feedback `Kf = 0.1` and `Tf = 1.0` from the DC1C sample data in IEEE 421.5-2016, Annex H, because the generators lose synchronism without it. These values are not specific to this case. All four exciters use `Vrmax = 3.0` for consistency.
+
 Simulation and validation studies have not yet been implemented for this case.
 
 [^1]: PowerWorld, [Simulator Help](https://www.powerworld.com/WebHelp/).

cases/PhasorDynamics/TwoArea/TwoArea.case.json

@@ -681,8 +681,8 @@
         "Ta": 1.0,
         "Ke": 0.0,
         "Te": 0.0,
-        "Kf": 0.0,
-        "Tf": 0.0,
+        "Kf": 0.1,
+        "Tf": 1.0,
         "Vrmin": -3.0,
         "Vrmax": 3.0,
         "E1": 0.0,
@@ -706,8 +706,8 @@
         "Ta": 1.0,
         "Ke": 0.0,
         "Te": 0.0,
-        "Kf": 0.0,
-        "Tf": 0.0,
+        "Kf": 0.1,
+        "Tf": 1.0,
         "Vrmin": -3.0,
         "Vrmax": 3.0,
         "E1": 0.0,
@@ -731,10 +731,10 @@
         "Ta": 1.0,
         "Ke": 0.0,
         "Te": 0.0,
-        "Kf": 0.0,
-        "Tf": 0.0,
+        "Kf": 0.1,
+        "Tf": 1.0,
         "Vrmin": -3.0,
-        "Vrmax": 0.0,
+        "Vrmax": 3.0,
         "E1": 0.0,
         "E2": 0.0,
         "Se1": 0.0,
@@ -756,8 +756,8 @@
         "Ta": 1.0,
         "Ke": 0.0,
         "Te": 0.0,
-        "Kf": 0.0,
-        "Tf": 0.0,
+        "Kf": 0.1,
+        "Tf": 1.0,
         "Vrmin": -3.0,
         "Vrmax": 3.0,
         "E1": 0.0,