Prohibit collect-formula & collect-expression mix

Mixing these instructions is not handled by the analysis, so the initializer validates the input for this invalid setup. Issue #183
rakhimov · May 11, 2017 · 9c341c2 · 9c341c2
1 parent 48bb5e7
commit 9c341c2
Show file tree

Hide file tree

Showing 8 changed files with 188 additions and 3 deletions.
diff --git a/doc/event_tree_analysis.rst b/doc/event_tree_analysis.rst
@@ -29,5 +29,9 @@ Event tree validation notes:
       In expressions evaluating a functional event state,
       the state must be one of possible states declared in functional event forks.
 
+    - The event-tree is exclusively defined by either formulas or expressions,
+      but no mix is allowed.
+
     - In general (fault-tree linking, event-tree linking),
-      there's no computationally cheap way to validate conditional independence of branches.
+      the validation of mutual-exclusivity, completeness (sum to 1), or conditional-independence
+      is not performed.
diff --git a/doc/opsa_support.rst b/doc/opsa_support.rst
@@ -91,6 +91,7 @@ Deviations from the Open-PSA MEF
 - Recursive parameters are not allowed.
 - Recursive event-tree rules (instructions) are not allowed.
 - Recursive event-tree links (instructions) are not allowed.
+- Mixing collect-expression and collect-formula is not allowed.
 - An empty text in XML attributes or elements is considered an error.
 
 

diff --git a/src/initializer.cc b/src/initializer.cc
@@ -1184,6 +1184,19 @@ void Initializer::ValidateInitialization() {
   // The cycles in links are checked only after ensuring their valid locations.
   cycle::CheckCycle<Link>(links_, "event-tree link");
 
+  // Event-tree instruction homogeneity checks only after cycle checks.
+  for (const EventTreePtr& event_tree : model_->event_trees()) {
+    try {
+      for (const NamedBranchPtr& branch : event_tree->branches()) {
+        EnsureHomogeneousEventTree(*branch);  // No mixed instructions.
+      }
+      EnsureHomogeneousEventTree(event_tree->initial_state());
+    } catch (ValidationError& err) {
+      err.msg("In event tree " + event_tree->name() + ", " + err.msg());
+      throw;
+    }
+  }
+
   // Check if all basic events have expressions for probability analysis.
   if (settings_.probability_analysis()) {
     std::string msg;
@@ -1249,7 +1262,7 @@ void Initializer::EnsureLinksOnlyInSequences(const Branch& branch) {
     }
   };
 
-  struct CheckLink {
+  struct {
     void operator()(Sequence*) {}
     void operator()(const NamedBranch*) {}
 
@@ -1269,6 +1282,59 @@ void Initializer::EnsureLinksOnlyInSequences(const Branch& branch) {
   link_checker(&branch);
 }
 
+void Initializer::EnsureHomogeneousEventTree(const Branch& branch) {
+  enum Type { kUnknown, kExpression, kFormula };
+
+  struct Visitor : public NullVisitor {
+    void Visit(const CollectExpression*) override {
+      switch (type) {
+        case kFormula:
+          throw ValidationError("Mixed collect-expression and collect-formula");
+        case kUnknown:
+          type = kExpression;
+        case kExpression:
+          break;
+      }
+    }
+
+    void Visit(const CollectFormula*) override {
+      switch (type) {
+        case kExpression:
+          throw ValidationError("Mixed collect-expression and collect-formula");
+        case kUnknown:
+          type = kFormula;
+        case kFormula:
+          break;
+      }
+    }
+
+    void Visit(const Link* link) override {
+      (*this)(&link->event_tree().initial_state());
+    }
+
+    void CheckInstructions(const std::vector<Instruction*>& instructions) {
+      for (const Instruction* instruction : instructions)
+        instruction->Accept(this);
+    }
+
+    void operator()(const Sequence* sequence) {
+      CheckInstructions(sequence->instructions());
+    }
+    void operator()(const Branch* arg_branch) {
+      CheckInstructions(arg_branch->instructions());
+      boost::apply_visitor(*this, arg_branch->target());
+    }
+    void operator()(const Fork* fork) {
+      for (const Path& fork_path : fork->paths())
+        (*this)(&fork_path);
+    }
+
+    Type type = kUnknown;
+  } homogeneous_checker;
+
+  homogeneous_checker(&branch);
+}
+
 void Initializer::ValidateExpressions() {
   // Check for cycles in parameters.
   // This must be done before expressions.

diff --git a/src/initializer.h b/src/initializer.h
@@ -336,6 +336,8 @@ class Initializer : private boost::noncopyable {
   /// @param[in] branch  The event tree branch to start the check.
   ///
   /// @throws ValidationError  The order of forks is invalid.
+  ///
+  /// @pre All named branches are fed separately from initial states.
   void CheckFunctionalEventOrder(const Branch& branch);
 
   /// Checks that link instructions are used only in event-tree sequences.
@@ -347,6 +349,16 @@ class Initializer : private boost::noncopyable {
   /// @pre All named branches are fed separately from initial states.
   void EnsureLinksOnlyInSequences(const Branch& branch);
 
+  /// Ensures that event-tree does not mix
+  /// collect-expression and collect-formula.
+  ///
+  /// @param[in] branch  The event tree branch to start the check.
+  ///
+  /// @throws ValidationError  The Link instruction is misused.
+  ///
+  /// @pre All named branches are fed separately from initial states.
+  void EnsureHomogeneousEventTree(const Branch& branch);
+
   /// Validates expressions and anything
   /// that is dependent on them,
   /// such as parameters and basic events.

diff --git a/tests/initializer_tests.cc b/tests/initializer_tests.cc
@@ -134,7 +134,10 @@ TEST(InitializerTest, IncorrectEtaInputs) {
       "invalid_link_instruction.xml",
       "invalid_link_in_branch.xml",
       "invalid_link_in_rule.xml",
-      "undefined_arg_collect_formula.xml"};
+      "undefined_arg_collect_formula.xml",
+      "mixing_collect_instructions.xml",
+      "mixing_collect_instructions_link.xml",
+      "mixing_collect_instructions_fork.xml"};
   for (const auto& input : incorrect_inputs) {
     EXPECT_THROW(Initializer({dir + input}, core::Settings()), ValidationError)
         << " Filename: " << input;

diff --git a/tests/input/eta/mixing_collect_instructions.xml b/tests/input/eta/mixing_collect_instructions.xml
@@ -0,0 +1,27 @@
+<?xml version="1.0"?>
+
+<opsa-mef>
+  <define-initiating-event name="Trigger" event-tree="InstructionMix"/>
+  <define-event-tree name="InstructionMix">
+    <define-functional-event name="F"/>
+    <define-sequence name="S"/>
+    <initial-state>
+      <fork functional-event="F">
+        <path state="on">
+          <collect-formula>
+            <basic-event name="B"/>
+          </collect-formula>
+          <collect-expression>
+            <float value="0.1"/>
+          </collect-expression>
+          <sequence name="S"/>
+        </path>
+      </fork>
+    </initial-state>
+  </define-event-tree>
+  <model-data>
+    <define-basic-event name="B">
+      <float value="0.5"/>
+    </define-basic-event>
+  </model-data>
+</opsa-mef>
diff --git a/tests/input/eta/mixing_collect_instructions_fork.xml b/tests/input/eta/mixing_collect_instructions_fork.xml
@@ -0,0 +1,32 @@
+<?xml version="1.0"?>
+
+<opsa-mef>
+  <define-initiating-event name="Trigger" event-tree="InstructionMix"/>
+  <define-event-tree name="InstructionMix">
+    <define-functional-event name="F"/>
+    <define-functional-event name="G"/>
+    <define-sequence name="S"/>
+    <initial-state>
+      <fork functional-event="F">
+        <path state="on">
+          <collect-formula>
+            <basic-event name="B"/>
+          </collect-formula>
+          <fork functional-event="G">
+            <path state="on">
+              <collect-expression>
+                <float value="0.1"/>
+              </collect-expression>
+              <sequence name="S"/>
+            </path>
+          </fork>
+        </path>
+      </fork>
+    </initial-state>
+  </define-event-tree>
+  <model-data>
+    <define-basic-event name="B">
+      <float value="0.5"/>
+    </define-basic-event>
+  </model-data>
+</opsa-mef>
diff --git a/tests/input/eta/mixing_collect_instructions_link.xml b/tests/input/eta/mixing_collect_instructions_link.xml
@@ -0,0 +1,40 @@
+<?xml version="1.0"?>
+
+<opsa-mef>
+  <define-initiating-event name="Trigger" event-tree="InstructionMix"/>
+  <define-event-tree name="ExpressionOnly">
+    <define-functional-event name="G"/>
+    <define-sequence name="SE"/>
+    <initial-state>
+      <fork functional-event="G">
+        <path state="on">
+          <collect-expression>
+            <float value="0.1"/>
+          </collect-expression>
+          <sequence name="SE"/>
+        </path>
+      </fork>
+    </initial-state>
+  </define-event-tree>
+  <define-event-tree name="InstructionMix">
+    <define-functional-event name="F"/>
+    <define-sequence name="S">
+      <event-tree name="ExpressionOnly"/>
+    </define-sequence>
+    <initial-state>
+      <fork functional-event="F">
+        <path state="on">
+          <collect-formula>
+            <basic-event name="B"/>
+          </collect-formula>
+          <sequence name="S"/>
+        </path>
+      </fork>
+    </initial-state>
+  </define-event-tree>
+  <model-data>
+    <define-basic-event name="B">
+      <float value="0.5"/>
+    </define-basic-event>
+  </model-data>
+</opsa-mef>