Add warning when Operator.NONE is used for a connected port

And small changes based on PR feedback

libmuscle/cpp/src/libmuscle/mmsf_validator.cpp

@@ -45,13 +45,23 @@ MMSFValidator::MMSFValidator(PortManager const& port_manager, Logger & logger)
         connected_ports_[op] = connected_ports;
     }
 
+    if (!connected_ports_[Operator::NONE].empty())
+        logger_.warning(
+                "This instance is using ports with Operator.NONE. This does not "
+                "adhere to the Multiscale Modelling and Simulation Framework "
+                "and may lead to deadlocks. You can disable this warning by "
+                "setting the flag InstanceFlags::SKIP_MMSF_SEQUENCE_CHECKS "
+                "when creating the libmuscle::Instance.");
+
     // Allowed operator transitions, the following are unconditionally allowed
     allowed_transitions_[Operator::NONE] = {Operator::NONE, Operator::F_INIT};
     allowed_transitions_[Operator::F_INIT] = {Operator::O_I, Operator::O_F};
     allowed_transitions_[Operator::O_I] = {Operator::S};
     allowed_transitions_[Operator::S] = {Operator::O_I, Operator::O_F};
     allowed_transitions_[Operator::O_F] = {Operator::NONE};
     // If there are operators without connected ports, we can skip over those
+    // This logic is transitive, i.e. when there are no connected ports for both
+    // F_INIT and O_I, we will also add NONE -> S to self._allowed_transition:
     for (auto const op : {Operator::F_INIT, Operator::O_I, Operator::S, Operator::O_F}) {
         if (connected_ports_[op].empty()) {
             // Find all transitions A -> op -> B and allow transition A -> B:
@@ -61,8 +71,7 @@ MMSFValidator::MMSFValidator(PortManager const& port_manager, Logger & logger)
                 if (!contains(allowed, op))
                     continue;  // op is not in the allowed list
                 for (auto const & to_op : allowed_transitions_[op]) {
-                    // add to_op to allowed, if it is not already in the list:
-                    if (std::find(allowed.begin(), allowed.end(), to_op) == allowed.end())
+                    if (!contains(allowed, to_op))
                         allowed.push_back(to_op);
                 }
             }
@@ -85,18 +94,18 @@ MMSFValidator::MMSFValidator(PortManager const& port_manager, Logger & logger)
 void MMSFValidator::check_send(
         std::string const& port_name, Optional<int> slot)
 {
-    check_send_receive(port_name, slot);
+    check_send_receive_(port_name, slot);
 }
 
 void MMSFValidator::check_receive(
         std::string const& port_name, Optional<int> slot)
 {
-    check_send_receive(port_name, slot);
+    check_send_receive_(port_name, slot);
 }
 
 void MMSFValidator::reuse_instance() {
     if (enabled_) {
-        check_transition(Operator::NONE, "");
+        check_transition_(Operator::NONE, "");
     }
 }
 
@@ -106,32 +115,29 @@ void MMSFValidator::skip_f_init() {
     current_ports_used_ = connected_ports_[Operator::F_INIT];
 }
 
-void MMSFValidator::check_send_receive(
+void MMSFValidator::check_send_receive_(
         std::string const& port_name, Optional<int> slot)
 {
     if (!enabled_) return;
 
     auto op = port_operators_[port_name];
     if (current_operator_ != op) {
         // Operator changed, check that all ports were used in the previous operator
-        check_transition(op, port_name);
+        check_transition_(op, port_name);
     }
 
-    if (std::find(
-            current_ports_used_.begin(),
-            current_ports_used_.end(),
-            port_name) != current_ports_used_.end()) {
+    if (contains(current_ports_used_, port_name)) {
         // We're using the same port for a second time, this is fine if:
         // 1. We're allowed to do this operator immediately again, and
         // 2. All ports of the current operator have been used
         // Both are checked by check_transition_:
-        check_transition(op, port_name);
+        check_transition_(op, port_name);
     }
 
     current_ports_used_.push_back(port_name);
 }
 
-void MMSFValidator::check_transition(
+void MMSFValidator::check_transition_(
         ::ymmsl::Operator op, std::string const& port_name)
 {
     std::ostringstream expected_oss;
@@ -196,8 +202,8 @@ void MMSFValidator::check_transition(
                 ".\n"
                 "Not adhering to the Multiscale Modelling and Simulation Framework "
                 "may lead to deadlocks. You can disable this warning by "
-                "setting the flag InstanceFlags.SKIP_MMSF_SEQUENCE_CHECKS "
-                "when creating the libmuscle.Instance.");
+                "setting the flag InstanceFlags::SKIP_MMSF_SEQUENCE_CHECKS "
+                "when creating the libmuscle::Instance.");
     }
 
     current_operator_ = op;

libmuscle/cpp/src/libmuscle/mmsf_validator.hpp

@@ -50,7 +50,7 @@ class MMSFValidator {
 
     private:
         /** Actual implementation of check_send/check_receive. */
-        void check_send_receive(std::string const & port_name, Optional<int> slot);
+        void check_send_receive_(std::string const & port_name, Optional<int> slot);
         /** Check that a transition to the provided operator is allowed.
          * 
          * Log a warning when the transition does not adhere to the MMSF.
@@ -59,7 +59,7 @@ class MMSFValidator {
          * @param port_name The name of the port that was sent/receveived on. This is only
          *      used for constructing the warning message.
          */
-        void check_transition(::ymmsl::Operator op, std::string const & port_name);
+        void check_transition_(::ymmsl::Operator op, std::string const & port_name);
 
         PortManager const & port_manager_;
         Logger & logger_;

libmuscle/python/libmuscle/mmsf_validator.py

@@ -50,6 +50,14 @@ def __init__(self, port_manager: PortManager) -> None:
                 for operator, ports in port_names.items()
                 for port in ports}
 
+        if self._connected_ports.get(Operator.NONE, []):
+            _logger.warning(
+                    "This instance is using ports with Operator.NONE. This does not "
+                    "adhere to the Multiscale Modelling and Simulation Framework "
+                    "and may lead to deadlocks. You can disable this warning by "
+                    "setting the flag InstanceFlags.SKIP_MMSF_SEQUENCE_CHECKS "
+                    "when creating the libmuscle.Instance.")
+
         # Allowed operator transitions, the following are unconditionally allowed:
         self._allowed_transitions = {
                 Operator.NONE: [Operator.NONE, Operator.F_INIT],
@@ -58,6 +66,8 @@ def __init__(self, port_manager: PortManager) -> None:
                 Operator.S: [Operator.O_I, Operator.O_F],
                 Operator.O_F: [Operator.NONE]}
         # If there are operators without connected ports, we can skip over those
+        # This logic is transitive, i.e. when there are no connected ports for both
+        # F_INIT and O_I, we will also add NONE -> S to self._allowed_transition:
         for operator in [Operator.F_INIT, Operator.O_I, Operator.S, Operator.O_F]:
             if not self._connected_ports.get(operator, []):
                 # Find all transitions A -> operator -> B and allow transition A -> B: