From 7e9fd46fca8cbb7a9bb13154842def38b7d3dd73 Mon Sep 17 00:00:00 2001 From: Maarten Sebregts Date: Wed, 28 Aug 2024 09:53:48 +0200 Subject: [PATCH] Add warning when Operator.NONE is used for a connected port And small changes based on PR feedback --- .../cpp/src/libmuscle/mmsf_validator.cpp | 36 +++++++++++-------- .../cpp/src/libmuscle/mmsf_validator.hpp | 4 +-- libmuscle/python/libmuscle/mmsf_validator.py | 10 ++++++ 3 files changed, 33 insertions(+), 17 deletions(-) diff --git a/libmuscle/cpp/src/libmuscle/mmsf_validator.cpp b/libmuscle/cpp/src/libmuscle/mmsf_validator.cpp index 3c34e2cc..037c3c46 100644 --- a/libmuscle/cpp/src/libmuscle/mmsf_validator.cpp +++ b/libmuscle/cpp/src/libmuscle/mmsf_validator.cpp @@ -45,6 +45,14 @@ 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}; @@ -52,6 +60,8 @@ MMSFValidator::MMSFValidator(PortManager const& port_manager, Logger & logger) 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 slot) { - check_send_receive(port_name, slot); + check_send_receive_(port_name, slot); } void MMSFValidator::check_receive( std::string const& port_name, Optional 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,7 +115,7 @@ 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 slot) { if (!enabled_) return; @@ -114,24 +123,21 @@ void MMSFValidator::check_send_receive( 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; diff --git a/libmuscle/cpp/src/libmuscle/mmsf_validator.hpp b/libmuscle/cpp/src/libmuscle/mmsf_validator.hpp index 9a7720f3..5c15fb54 100644 --- a/libmuscle/cpp/src/libmuscle/mmsf_validator.hpp +++ b/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 slot); + void check_send_receive_(std::string const & port_name, Optional 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_; diff --git a/libmuscle/python/libmuscle/mmsf_validator.py b/libmuscle/python/libmuscle/mmsf_validator.py index 1f85aeb7..ab69aa71 100644 --- a/libmuscle/python/libmuscle/mmsf_validator.py +++ b/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: