feat: Multiple inputs to vector-type parameters (#2189)

Co-authored-by: Tristan Youngs <trisyoungs@googlemail.com>

Author: trisyoungs

src/expression/value.h

@@ -35,9 +35,9 @@ class ExpressionValue : public Serialisable<>
     ValueType type_;
     // Whether current result type is fixed
     bool typeFixed_;
-    // Integer value (if type_ == IntegerType)
+    // Integer value (if storedDataType_ == IntegerType)
     int valueI_;
-    // Double value (if type_ == DoubleType)
+    // Double value (if storedDataType_ == DoubleType)
     double valueD_;
 
     public:

src/gui/models/nodeGraph/parameterModel.cpp

@@ -29,15 +29,15 @@ QVariant ParameterModel::data(const QModelIndex &index, int role) const
         case DESCRIPTION:
             return QString::fromStdString(std::string(it->second->description()));
         case DATA:
-            if (it->second->type() == typeid(Number))
-                return QVariant::fromValue(it->second->upcast<Number>()->get().asInteger());
-            if (it->second->type() == typeid(bool))
-                return QVariant::fromValue(it->second->upcast<bool>()->get());
+            if (it->second->storedDataType() == typeid(Number))
+                return QVariant::fromValue(it->second->get<Number>().asInteger());
+            if (it->second->storedDataType() == typeid(bool))
+                return QVariant::fromValue(it->second->get<bool>());
             return QString::fromStdString("Unrepresentable");
         case TYPE:
-            if (it->second->type() == typeid(Number))
+            if (it->second->storedDataType() == typeid(Number))
                 return "number";
-            if (it->second->type() == typeid(bool))
+            if (it->second->storedDataType() == typeid(bool))
                 return "bool";
             return "unknown";
 

src/nodes/atomicSpecies.cpp

@@ -7,7 +7,7 @@ AtomicSpeciesNode::AtomicSpeciesNode(Graph *parentGraph, Elements::Element Z) :
     at->interactionPotential().setFormAndParameters(ShortRangeFunctions::Form::LennardJones, "epsilon=0.3 sigma=2.0");
     species_.addAtom(Z, {}, 0.0, at);
 
-    addPointerOutput<const Species *>("Species", "Atomic species", species_);
+    addPointerOutput<const Species>("Species", "Atomic species", species_);
 }
 
 std::string_view AtomicSpeciesNode::type() const { return "AtomicSpecies"; }

src/nodes/configuration.cpp

@@ -5,7 +5,7 @@
 
 ConfigurationNode::ConfigurationNode(Graph *parentGraph) : Node(parentGraph)
 {
-    addPointerOutput<Configuration *>("Configuration", "Configuration object", configuration_);
+    addPointerOutput<Configuration>("Configuration", "Configuration object", configuration_);
 }
 
 std::string_view ConfigurationNode::type() const { return "Configuration"; }

src/nodes/edge.cpp

@@ -112,7 +112,7 @@ std::unique_ptr<Edge> Edge::create(Graph *parent, const EdgeDefinition &definiti
     }
 
     // Check that types are compatible
-    if (sourceOutput->type() != targetInput->type())
+    if (!targetInput->acceptsOutput(sourceOutput.get()))
         return {};
 
     // Create the edge
@@ -135,7 +135,7 @@ const ParameterBase &Edge::sourceOutput() const { return sourceOutput_; }
 Node &Edge::targetNode() const { return targetNode_; }
 
 // Return target input parameter
-const ParameterBase &Edge::targetInput() const { return targetInput_; }
+ParameterBase &Edge::targetInput() const { return targetInput_; }
 
 // Return definition for the edge
 EdgeDefinition Edge::definition() const
@@ -223,11 +223,18 @@ NodeConstants::ProcessResult Edge::pull()
     return NodeConstants::ProcessResult::Unchanged;
 }
 
+// Ensure next call to pull() will retrieve the data from the source node
+void Edge::forceNextPull() { sourceNodeVersionIndex_ = NodeConstants::InvalidVersion; }
+
+/*
+ * I/O
+ */
+
 // Express as a serialisable value
 SerialisedValue Edge::serialise() const { return definition().serialise(); }
 
-// Read values from a serialisable value This is required for the
-// SerialableValue type implementation, but we actually deserialise
+// Read values from a serialisable value. This is required for the
+// SerialisableValue type implementation, but we actually deserialise
 // Edges through an EdgeConnection.  I've added this error to
 // immediately alert us in case this function is ever called.
 void Edge::deserialise(const SerialisedValue &node)

src/nodes/edge.h

@@ -66,11 +66,13 @@ class Edge : public Serialisable<>
     // Return target node
     Node &targetNode() const;
     // Return target input parameter
-    const ParameterBase &targetInput() const;
+    ParameterBase &targetInput() const;
     // Return definition for the edge
     EdgeDefinition definition() const;
     // Pull the data from the source node to the target, returning a ProcessResult
     NodeConstants::ProcessResult pull();
+    // Ensure next call to pull() will retrieve the data from the source node
+    void forceNextPull();
 
     /*
      * I/O

src/nodes/gr/gr.cpp

@@ -25,7 +25,7 @@ GRNode::GRNode(Graph *parentGraph) : Node(parentGraph)
         internalTest_);
     addOption<GRNode::PartialsMethod>("Method", "Calculation method for partial radial distribution functions",
                                       partialsMethod_);
-    addOptionalPointerOutput<PartialSet *>("UnweightedGR", "Unweighted partials for target configuration", unweightedGR_);
+    addOptionalPointerOutput<PartialSet>("UnweightedGR", "Unweighted partials for target configuration", unweightedGR_);
 }
 
 // Return enum option info for NormalisationType

src/nodes/graph.cpp

@@ -192,7 +192,16 @@ bool Graph::removeEdge(Edge *edgeToRemove)
         std::find_if(edges_.begin(), edges_.end(), [edgeToRemove](const auto &edge) { return edge.get() == edgeToRemove; });
     if (it == edges_.end())
         return Messenger::error("Edge pointer doesn't exist, so can't remove it.\n");
+
+    // Need to flag the node containing the connected input that it is now out-of-date
+    auto &input = it->get()->targetInput();
+    input.setParentUpdateRequired();
+    if (input.isVector())
+        input.invalidateVector();
+
+    // Can now erase it
     edges_.erase(it);
+
     return true;
 }
 

src/nodes/node.cpp

@@ -66,9 +66,20 @@ void Node::setUpdateRequired()
     upToDate_ = false;
 
     // Make sure all output edges propagate this information down
-    for (auto &&[outputName, edge] : outputEdges())
-        if (!edge->targetInput().flags().isSet(ParameterBase::ParameterFlags::NoUpdate))
-            edge->targetInput().setParentUpdateRequired();
+    for (auto &&[outputName, edges] : outputEdges())
+        for (auto edge : edges)
+        {
+            auto &input = edge->targetInput();
+
+            if (input.flags().isSet(ParameterBase::ParameterFlags::NoUpdate))
+                continue;
+
+            input.setParentUpdateRequired();
+
+            // If the target input is a vector, all edges to it must be marked for re-pull and its data cleared
+            if (input.isVector())
+                input.invalidateVector();
+        }
 }
 
 // Return whether the node's data is up-to-date
@@ -82,8 +93,9 @@ bool Node::inputsAreValid() const
         // Does this input have a link or links?
         if (inputEdges_.contains(inputName))
         {
-            if (!inputEdges_.at(inputName)->sourceOutput().parent()->inputsAreValid())
-                return false;
+            for (const auto edge : inputEdges_.at(inputName))
+                if (!edge->sourceOutput().parent()->inputsAreValid())
+                    return false;
         }
         else if (parameter->flags().isSet(ParameterBase::ParameterFlags::Required))
             return false;
@@ -95,18 +107,20 @@ bool Node::inputsAreValid() const
 // Run the node, retrieving dependent inputs as necessary
 NodeConstants::ProcessResult Node::run()
 {
-    // Check our input links - if any are out-of-date we must retrieve new values. This will automatically unset upToDate_
-    for (auto &[inputName, edge] : inputEdges_)
+    // Pull all input edges. If any are out-of-date and get re-set this will automatically unset upToDate_
+    for (auto &[inputName, edges] : inputEdges_)
     {
-        auto edgeResult = edge->pull();
-        switch (edgeResult)
+        for (const auto edge : edges)
         {
-            case (NodeConstants::ProcessResult::Failed):
-            case (NodeConstants::ProcessResult::InputsNotSatisfied):
-                return NodeConstants::ProcessResult::Failed;
-            case (NodeConstants::ProcessResult::Success):
-            case (NodeConstants::ProcessResult::Unchanged):
-                break;
+            switch (edge->pull())
+            {
+                case (NodeConstants::ProcessResult::Failed):
+                case (NodeConstants::ProcessResult::InputsNotSatisfied):
+                    return NodeConstants::ProcessResult::Failed;
+                case (NodeConstants::ProcessResult::Success):
+                case (NodeConstants::ProcessResult::Unchanged):
+                    break;
+            }
         }
     }
 
@@ -143,58 +157,74 @@ NodeConstants::ProcessResult Node::process() { return NodeConstants::ProcessResu
 // Link edge, returning whether we accept it
 bool Node::linkEdge(Edge *edge)
 {
-
     // The supplied Edge was created via our parent Graph, but we will still check to see whether we accept it
     if (&edge->targetNode() == this)
     {
         // We are the target node, so we will double-check the specified input to see if it can accept the connection
-        // Simple check at present, we accept at most one connection per input, so if one already exists we complain
+        // We accept one connection per input in the case of non-vector parameters, so if one already exists we complain.
+        // Vector inputs are currently unbounded.
         if (inputEdges_.contains(edge->targetInput().name()))
-            return Messenger::error("Node '{}' refusing to accept Edge connecting to input '{}' as one already exists.\n",
-                                    name(), edge->targetInput().name());
+        {
+            // Already have input edges to this parameter, so check current size and type
+            if (!inputEdges_.at(edge->targetInput().name()).empty())
+            {
+                if (edge->targetInput().nAllowedInputEdges() != ParameterBase::AllowedEdgeCount::AnyNumber)
+                    return Messenger::error("Node '{}' refusing to accept Edge connecting to input '{}' as it already has the "
+                                            "maximum permissible.\n",
+                                            name(), edge->targetInput().name());
+            }
+        }
 
         // All good, so add the input to our list
-        inputEdges_[edge->targetInput().name()] = edge;
+        inputEdges_[edge->targetInput().name()].push_back(edge);
 
         // Adding an Edge to an input always invalidates the target
         invalidate();
     }
     else if (&edge->sourceNode() == this)
     {
         // We are the source node - add the outgoing edge to our list
-        outputEdges_[edge->sourceOutput().name()] = edge;
+        outputEdges_[edge->sourceOutput().name()].push_back(edge);
     }
     else
         return Messenger::error("Node '{}' is neither the source nor the target for the supplied Edge.\n", name());
 
     return true;
 }
 
+// Erase the specified edge from the given map, returning if it was found and erased
+bool Node::eraseEdge(EdgeMap &map, Edge *edge)
+{
+    auto mapIt = std::find_if(map.begin(), map.end(),
+                              [&](auto &edges)
+                              {
+                                  auto edgeIt = std::find(edges.second.begin(), edges.second.end(), edge);
+                                  if (edgeIt != edges.second.end())
+                                  {
+                                      edges.second.erase(edgeIt);
+                                      return true;
+                                  }
+                                  return edgeIt != edges.second.end();
+                              });
+    return mapIt != map.end();
+}
+
 // Unlink edge
 void Node::unlinkEdge(Edge *edge)
 {
     // If we are the Edge's targetNode_ then we should have its pointer in inputEdges_
     if (&edge->targetNode() == this)
     {
-        auto it = std::find_if(inputEdges_.begin(), inputEdges_.end(),
-                               [edge](const auto &inputEdge) { return edge == inputEdge.second; });
-        if (it == inputEdges_.end())
-            Messenger::error("Tried to unlink an incoming edge to target node '{}' which knew nothing about it.\n", name());
-        else
-        {
-            inputEdges_.erase(it);
+        if (eraseEdge(inputEdges_, edge))
             invalidate();
-        }
+        else
+            Messenger::error("Tried to unlink an incoming edge to target node '{}' which knew nothing about it.\n", name());
     }
     else if (&edge->sourceNode() == this)
     {
         // We are the source node for the edge...
-        auto it = std::find_if(outputEdges_.begin(), outputEdges_.end(),
-                               [edge](const auto &outputEdge) { return edge == outputEdge.second; });
-        if (it == outputEdges_.end())
+        if (!eraseEdge(outputEdges_, edge))
             Messenger::error("Tried to unlink an outgoing edge from source node '{}' which knew nothing about it.\n", name());
-        else
-            outputEdges_.erase(it);
     }
     else
         Messenger::error("Node '{}' is neither the source nor the target for the Edge being unlinked.\n", name());
@@ -256,6 +286,16 @@ Node::EdgeMap &Node::inputEdges() { return inputEdges_; }
 // Get the outgoing edges from this node
 Node::EdgeMap &Node::outputEdges() { return outputEdges_; }
 
+// Mark incoming edges to the specified parameter as needing a re-pull
+void Node::markIncomingEdgesForPull(const ParameterBase *toParameter) const
+{
+    if (!inputEdges_.contains(toParameter->name()))
+        return;
+
+    for (const auto edge : inputEdges_.at(toParameter->name()))
+        edge->forceNextPull();
+}
+
 // Returns the node parent graph
 Graph *Node::parentGraph() const { return parentGraph_; }