core: reject malformed models instead of running them (#56, #57, #60, #64, #69)

include/numsim-materials/core/material_context.h

@@ -86,8 +86,19 @@ class material_context {
     m_engine.update_property(material, property, exclude);
   }
 
-  void commit() { m_engine.commit(); }
-  void revert() { m_engine.revert(); }
+  /// Advance / roll back history. The driver owns these calls -- materials and
+  /// solvers never make them. Both need the same finalize() gate their siblings
+  /// have: the engine's history list is built by finalize(), so on an
+  /// unfinalized context they used to be silent no-ops rather than errors.
+  void commit() {
+    check_finalized("commit");
+    m_engine.commit();
+  }
+
+  void revert() {
+    check_finalized("revert");
+    m_engine.revert();
+  }
 
   // --- Checkpoint / Restart ---
 

include/numsim-materials/core/object_store.h

@@ -1,9 +1,11 @@
 #ifndef OBJECT_STORE_H
 #define OBJECT_STORE_H
 
+#include <functional>
 #include <memory>
 #include <stdexcept>
 #include <string>
+#include <string_view>
 #include <unordered_map>
 #include <vector>
 #include "numsim-materials/core/material_base.h"
@@ -36,6 +38,7 @@ class object_store {
   template <typename Material>
   Material& create(parameter_handler& params,
                    property_handler& properties, material_handler& materials) {
+    check_name_available(params);
     auto ptr = std::make_unique<Material>(params, properties, materials);
     auto& ref = *ptr;
     adopt(std::move(ptr), materials);
@@ -47,6 +50,7 @@ class object_store {
                                   parameter_handler& params,
                                   property_handler& properties,
                                   material_handler& materials) {
+    check_name_available(params);
     auto ptr = factory_type::instance().create(
         type_name, params, properties, materials);
     auto& ref = *ptr;
@@ -55,8 +59,12 @@ class object_store {
   }
 
   /// Find by name.
+  ///
+  /// Transparent lookup: hashing a string_view directly keeps this allocation
+  /// free, which is what lets it be noexcept. Building a std::string for the
+  /// key could throw bad_alloc, and under noexcept that is std::terminate.
   material_interface_type* find(std::string_view name) const noexcept {
-    auto it = m_by_name.find(std::string(name));
+    auto it = m_by_name.find(name);
     return it != m_by_name.end() ? it->second : nullptr;
   }
 
@@ -65,6 +73,32 @@ class object_store {
   }
 
 private:
+  /// Reject a duplicate name before anything is constructed.
+  ///
+  /// Names are the only way materials address each other -- every input wire,
+  /// every material_ref and every property lookup resolves against one. A
+  /// second material under a live name used to overwrite the first in the name
+  /// index and in the material handler, leaving it owned by m_storage but
+  /// unreachable: consumers wired before the collision kept the first,
+  /// consumers wired after silently got the second. With mismatched property
+  /// types that is not a wrong answer but a wrong pointer, because lookups
+  /// resolve through a static_cast -- a consumer expecting a tensor writes
+  /// through a pointer to a double. ASan reports a heap-buffer-overflow;
+  /// without ASan it is silent.
+  ///
+  /// Checked here, before construction, rather than in adopt(): a material
+  /// registers its properties under its own name while constructing, so by the
+  /// time adopt() sees it the collision has already happened in the property
+  /// registry.
+  void check_name_available(const parameter_handler& params) const {
+    const auto& name = params.template get<std::string>("name");
+    if (m_by_name.contains(name))
+      throw std::invalid_argument(
+          "object_store: a material named '" + name + "' already exists. "
+          "Material names must be unique -- they are what input wires, "
+          "material references and property lookups resolve against.");
+  }
+
   /// Take ownership of a fully constructed material and publish it.
   ///
   /// Registration happens HERE and not in material_base's constructor, because
@@ -84,9 +118,18 @@ class object_store {
     m_storage.push_back(std::move(ptr));
   }
 
+  /// Lets find() look up by string_view without building a std::string.
+  struct transparent_string_hash {
+    using is_transparent = void;
+    std::size_t operator()(std::string_view s) const noexcept {
+      return std::hash<std::string_view>{}(s);
+    }
+  };
+
   std::vector<std::unique_ptr<material_interface_type>> m_storage;
   std::vector<material_interface_type*> m_interfaces;
-  std::unordered_map<std::string, material_interface_type*> m_by_name;
+  std::unordered_map<std::string, material_interface_type*,
+                     transparent_string_hash, std::equal_to<>> m_by_name;
 };
 
 } // namespace numsim::materials

include/numsim-materials/core/property_engine.h

@@ -45,7 +45,14 @@ class property_engine {
   void update_property(const std::string& material, const std::string& property,
                        const std::unordered_set<const property_base*>& exclude = {}) {
     auto* target = find_in_graph(material, property);
-    if (!target) return;
+    // Returning quietly here made a typo indistinguishable from a property that
+    // legitimately never changes: the driver asks for an update, gets no error,
+    // and reads a stale value for the rest of the analysis.
+    if (!target)
+      throw std::runtime_error(
+          "update_property(): property '" + material + "::" + property +
+          "' is not in the graph. Check the material and property names, and "
+          "that the material publishes this property.");
     update_property(target, exclude);
   }
 

include/numsim-materials/materials/tensor_component_stepper.h

@@ -1,6 +1,8 @@
 #ifndef TENSOR_COMPONENT_STEPPER_H
 #define TENSOR_COMPONENT_STEPPER_H
 
+#include <stdexcept>
+#include <string>
 #include <tmech/tmech.h>
 #include "numsim-materials/core/material_base.h"
 
@@ -35,7 +37,9 @@ class tensor_component_stepper
         m_tensor(base::template add_output<tensor>("strain", &tensor_component_stepper::update)),
         m_inc(base::template get_parameter<value_type>("increment")),
         m_indices(base::template get_parameter<indices>("indices"))
-  {}
+  {
+    validate_indices();
+  }
 
   /**
    * @brief Defines the parameters required by this class.
@@ -68,6 +72,30 @@ class tensor_component_stepper
   }
 
 private:
+  /// "indices" addresses a tensor component directly, so a deck typo used to
+  /// land in update() as an out-of-bounds subscript: [9,9] on a 3x3 is a read
+  /// and a write past the end of the tensor (ASan: heap-buffer-overflow), and
+  /// too few entries read m_indices[1] past the end of the vector itself.
+  /// Neither is checked by tmech, so this is the only place it can be caught.
+  /// Checked once here rather than in update(), which runs per increment.
+  void validate_indices() const {
+    if (m_indices.size() != Rank)
+      throw std::invalid_argument(
+          "tensor_component_stepper ('" + this->name() + "'): \"indices\" has " +
+          std::to_string(m_indices.size()) + " entr" +
+          (m_indices.size() == 1 ? "y" : "ies") + ", but a rank-" +
+          std::to_string(Rank) + " tensor needs exactly " +
+          std::to_string(Rank) + ".");
+
+    for (std::size_t i = 0; i < m_indices.size(); ++i)
+      if (m_indices[i] >= Dim)
+        throw std::invalid_argument(
+            "tensor_component_stepper ('" + this->name() + "'): \"indices\"[" +
+            std::to_string(i) + "] is " + std::to_string(m_indices[i]) +
+            ", which is out of range for a " + std::to_string(Dim) + "D tensor "
+            "(valid: 0.." + std::to_string(Dim - 1) + ").");
+  }
+
   /// Produced properties
   tensor &m_tensor;
 

tests/CMakeLists.txt

@@ -10,6 +10,7 @@ macro(add_numsim_test TARGET_NAME)
 endmacro()
 
 add_numsim_test(test_property_graph test_property_graph.cpp)
+add_numsim_test(test_model_validation test_model_validation.cpp)
 add_numsim_test(test_materials test_materials.cpp)
 add_numsim_test(test_damage test_damage.cpp)
 add_numsim_test(test_property_recorder test_property_recorder.cpp)