Merge pull request #146 from force-h2020/specify-kpi-at-mco-level

Specify KPIs at the MCO level

force_bdss/core/data_value.py

-from traits.api import HasStrictTraits, Any, String, Enum, Bool
+from traits.api import HasStrictTraits, Any, String, Enum
 
 
 class DataValue(HasStrictTraits):
@@ -22,9 +22,6 @@ class DataValue(HasStrictTraits):
     #: A flag for the quality of the data.
     quality = Enum("AVERAGE", "POOR", "GOOD")
 
-    # Set by the engine. True if the data value contains a KPI.
-    is_kpi = Bool(False)
-
     def __str__(self):
 
         s = "{} {} = {}".format(
@@ -35,7 +32,4 @@ class DataValue(HasStrictTraits):
 
         s += " ({})".format(str(self.quality))
 
-        if self.is_kpi:
-            s += " (KPI)"
-
         return s

force_bdss/core/kpi_specification.py

+from traits.api import Enum, HasStrictTraits
+
+from force_bdss.local_traits import Identifier
+
+
+class KPISpecification(HasStrictTraits):
+    #: The user defined name of the variable containing the kpi value.
+    name = Identifier()
+
+    #: The expected outcome of the procedure relative to this KPI.
+    objective = Enum("MINIMISE")

force_bdss/core/output_slot_info.py

-from traits.api import HasStrictTraits, Bool
+from traits.api import HasStrictTraits
 
 from ..local_traits import Identifier
 
@@ -13,7 +13,3 @@ class OutputSlotInfo(HasStrictTraits):
     """
     #: The user defined name of the variable containing the value.
     name = Identifier()
-
-    #: True if the value associated to this output slot must be exported as
-    #: a KPI.
-    is_kpi = Bool(False)

force_bdss/core_evaluation_driver.py

@@ -65,7 +65,15 @@ def execute_workflow(workflow, data_values):
         available_data_values += ds_results
 
     log.info("Aggregating KPI data")
-    kpi_results = [dv for dv in available_data_values if dv.is_kpi]
+
+    kpi_results = []
+    kpi_names = [kpi.name for kpi in workflow.mco.kpis]
+
+    kpi_results = [
+        dv
+        for dv in available_data_values
+        if dv.name in kpi_names
+    ]
 
     return kpi_results
 
@@ -165,7 +173,6 @@ def _compute_layer_results(environment_data_values,
         # Add the names as specified by the user.
         for dv, output_slot_info in zip(res, model.output_slot_info):
             dv.name = output_slot_info.name
-            dv.is_kpi = output_slot_info.is_kpi
 
         # If the name was not specified, simply discard the value,
         # because apparently the user is not interested in it.

force_bdss/core_mco_driver.py

@@ -88,17 +88,10 @@ class CoreMCODriver(BaseCoreDriver):
 
     @on_trait_change("mco:started")
     def _deliver_start_event(self):
-        output_kpis = []
-        for layer in self.workflow.execution_layers:
-            for data_source in layer.data_sources:
-                output_kpis.extend(
-                    info for info in data_source.output_slot_info
-                    if info.is_kpi
-                )
-
+        mco_model = self.workflow.mco
         self._deliver_event(MCOStartEvent(
-            input_names=tuple(p.name for p in self.workflow.mco.parameters),
-            output_names=tuple([on.name for on in output_kpis])
+            input_names=tuple(p.name for p in mco_model.parameters),
+            output_names=tuple([kpi.name for kpi in mco_model.kpis])
         ))
 
     @on_trait_change("mco:finished")

force_bdss/data_sources/tests/test_base_data_source_model.py

@@ -57,12 +57,10 @@ class TestBaseDataSourceModel(unittest.TestCase):
                     {
                         "__traits_version__": "4.6.0",
                         "name": "baz",
-                        "is_kpi": False
                     },
                     {
                         "__traits_version__": "4.6.0",
                         "name": "quux",
-                        "is_kpi": False
                     }
                 ]
             })

force_bdss/io/tests/test_workflow_reader.py

@@ -34,7 +34,8 @@ class TestWorkflowReader(unittest.TestCase):
                                       ".dummy_mco_parameter",
                                 "model_data": {}
                             }
-                        ]
+                        ],
+                        "kpis": []
                     },
                 },
                 "execution_layers": [
@@ -146,6 +147,8 @@ class TestModelCreationFailure(unittest.TestCase):
                                       ".probe_mco_parameter",
                                 "model_data": {}
                             }
+                        ],
+                        "kpis": [
                         ]
                     },
                 },

force_bdss/io/tests/test_workflow_writer.py

@@ -2,13 +2,18 @@ import json
 import unittest
 
 from six import StringIO
+try:
+    import mock
+except ImportError:
+    from unittest import mock
 
 from force_bdss.core.execution_layer import ExecutionLayer
+from force_bdss.core.kpi_specification import KPISpecification
 from force_bdss.io.workflow_reader import WorkflowReader
 from force_bdss.tests.dummy_classes.factory_registry_plugin import \
     DummyFactoryRegistryPlugin
 
-from force_bdss.io.workflow_writer import WorkflowWriter
+from force_bdss.io.workflow_writer import WorkflowWriter, traits_to_dict
 from force_bdss.core.workflow import Workflow
 
 
@@ -53,6 +58,9 @@ class TestWorkflowWriter(unittest.TestCase):
         wf.mco.parameters = [
             self.mco_parameter_factory.create_model()
         ]
+        wf.mco.kpis = [
+            KPISpecification()
+        ]
         wf.execution_layers = [
             ExecutionLayer(data_sources=[
                 self.data_source_factory.create_model(),
@@ -73,3 +81,9 @@ class TestWorkflowWriter(unittest.TestCase):
         wfreader = WorkflowReader(self.registry)
         wf_result = wfreader.read(fp)
         self.assertIsNone(wf_result.mco)
+
+    def test_traits_to_dict_no_version(self):
+        mock_traits = mock.Mock()
+        mock_traits.__getstate__ = mock.Mock(return_value={"foo": "bar"})
+
+        self.assertEqual(traits_to_dict(mock_traits), {"foo": "bar"})

force_bdss/io/workflow_reader.py

@@ -5,6 +5,7 @@ from traits.api import HasStrictTraits, Instance
 
 from force_bdss.core.execution_layer import ExecutionLayer
 from force_bdss.core.input_slot_info import InputSlotInfo
+from force_bdss.core.kpi_specification import KPISpecification
 from force_bdss.core.output_slot_info import OutputSlotInfo
 from force_bdss.core.workflow import Workflow
 from ..factory_registry_plugin import IFactoryRegistryPlugin
@@ -165,6 +166,9 @@ class WorkflowReader(HasStrictTraits):
         model_data["parameters"] = self._extract_mco_parameters(
             mco_id,
             model_data["parameters"])
+        model_data["kpis"] = self._extract_kpi_specifications(
+            model_data["kpis"]
+        )
 
         try:
             model = mco_factory.create_model(model_data)
@@ -281,6 +285,9 @@ class WorkflowReader(HasStrictTraits):
 
         return parameters
 
+    def _extract_kpi_specifications(self, info):
+        return [KPISpecification(**d) for d in info]
+
     def _extract_input_slot_info(self, info):
         return [InputSlotInfo(**d) for d in info]
 

force_bdss/io/workflow_writer.py

@@ -46,11 +46,7 @@ class WorkflowWriter(HasStrictTraits):
 
         parameters_data = []
         for param in data["model_data"]["parameters"]:
-            state = param.__getstate__()
-            try:
-                state.pop("__traits_version__")
-            except KeyError:
-                pass
+            state = traits_to_dict(param)
 
             parameters_data.append(
                 {
@@ -60,6 +56,15 @@ class WorkflowWriter(HasStrictTraits):
             )
 
         data["model_data"]["parameters"] = parameters_data
+
+        kpis_data = []
+        for kpi in data["model_data"]["kpis"]:
+            kpis_data.append(
+                traits_to_dict(kpi)
+            )
+
+        data["model_data"]["kpis"] = kpis_data
+
         return data
 
     def _execution_layer_data(self, layer):
@@ -75,13 +80,21 @@ class WorkflowWriter(HasStrictTraits):
         """
         Extracts the data from an external model and returns its dictionary
         """
-        state = model.__getstate__()
-        try:
-            state.pop("__traits_version__")
-        except KeyError:
-            pass
+        state = traits_to_dict(model)
 
         return {
             "id": model.factory.id,
             "model_data": state
         }
+
+
+def traits_to_dict(traits_obj):
+    """Converts a traits class into a dict, removing the pesky
+    traits version."""
+    state = traits_obj.__getstate__()
+    try:
+        state.pop("__traits_version__")
+    except KeyError:
+        pass
+
+    return state

force_bdss/mco/base_mco_model.py

 from traits.api import ABCHasStrictTraits, Instance, List
 
+from force_bdss.core.kpi_specification import KPISpecification
 from .parameters.base_mco_parameter import BaseMCOParameter
 from .i_mco_factory import IMCOFactory
 
@@ -17,9 +18,12 @@ class BaseMCOModel(ABCHasStrictTraits):
                        visible=False,
                        transient=True)
 
-    # A list of the parameters for the MCO
+    #: A list of the parameters for the MCO
     parameters = List(BaseMCOParameter, visible=False)
 
+    #: A list of KPI specification objects and their objective.
+    kpis = List(KPISpecification, visible=False)
+
     def __init__(self, factory, *args, **kwargs):
         self.factory = factory
         super(BaseMCOModel, self).__init__(*args, **kwargs)

force_bdss/tests/fixtures/test_csv.json

-{
-  "version": "1",
-  "workflow": {
-    "mco": {
-      "id": "force.bdss.enthought.factory.dummy_dakota",
-      "model_data": {
-        "parameters" : [
-          {
-            "id": "force.bdss.enthought.factory.dummy_dakota.parameter.ranged",
-            "model_data": {
-                "initial_value": 3,
-                "lower_bound": 0,
-                "upper_bound": 10,
-                "name": "p0",
-                "type": "PRESSURE"
-            }
-          }
-        ]
-      }
-    },
-    "data_sources": [
-      {
-        "id": "force.bdss.enthought.factory.csv_extractor",
-        "model_data": {
-          "filename": "foo.csv",
-          "row": 3,
-          "column": 5,
-          "cuba_type": "PRESSURE",
-          "input_slot_maps": [
-          ],
-          "output_slot_names": [
-            "p1"
-          ]
-        }
-      },
-      {
-        "id": "force.bdss.enthought.factory.csv_extractor",
-        "model_data": {
-          "filename": "foo.csv",
-          "row": 3,
-          "column": 5,
-          "cuba_type": "PRESSURE",
-          "input_slot_maps": [
-          ],
-          "output_slot_names": [
-            "p2"
-          ]
-        }
-      }
-    ],
-    "kpi_calculators": [
-      {
-        "id": "force.bdss.enthought.factory.kpi_adder",
-        "model_data": {
-          "cuba_type_in": "PRESSURE",
-          "cuba_type_out": "TOTAL_PRESSURE",
-          "input_slot_maps": [
-            {
-              "name": "p0"
-            },
-            {
-              "name": "p1"
-            },
-            {
-              "name": "p2"
-            }
-          ],
-          "output_slot_names": [
-            "ptot"
-          ]
-        }
-      }
-    ],
-    "notification_listeners": [
-    ]
-  }
-}

force_bdss/tests/fixtures/test_dummy.json

@@ -12,6 +12,12 @@
                             "type": "PRESSURE"
                         }
                     }
+                ],
+                "kpis": [
+                  {
+                    "name": "bar",
+                    "objective": "MINIMISE"
+                  }
                 ]
             }
         },
@@ -31,8 +37,7 @@
                         ],
                         "output_slot_info": [
                             {
-                                "name": "bar",
-                                "is_kpi": true
+                                "name": "bar"
                             }
                         ]
                     }

force_bdss/tests/fixtures/test_empty.json

@@ -4,8 +4,6 @@
     "mco": null,
     "execution_layers": [
     ],
-    "kpi_calculators": [
-    ],
     "notification_listeners": [
     ]
   }

force_bdss/tests/fixtures/test_null.json

@@ -5,6 +5,8 @@
       "id": "force.bdss.enthought.plugin.test.v0.factory.probe_mco",
       "model_data": {
         "parameters" : [
+        ],
+        "kpis": [
         ]
       }
     },

force_bdss/tests/fixtures/test_probe.json

@@ -12,6 +12,12 @@
                             "type": "PRESSURE"
                         }
                     }
+                ],
+                "kpis": [
+                    {
+                        "name": "bar",
+                        "objective": "MINIMISE"
+                    }
                 ]
             }
         },
@@ -28,8 +34,7 @@
                         ],
                         "output_slot_info": [
                             {
-                                "name": "bar",
-                                "is_kpi": true
+                                "name": "bar"
                             }
                         ]
                     }

force_bdss/tests/test_core_evaluation_driver.py

@@ -4,6 +4,7 @@ import testfixtures
 import six
 
 from force_bdss.core.execution_layer import ExecutionLayer
+from force_bdss.core.kpi_specification import KPISpecification
 from force_bdss.core.output_slot_info import OutputSlotInfo
 from force_bdss.core.workflow import Workflow
 from force_bdss.tests.probe_classes.factory_registry_plugin import \
@@ -14,6 +15,7 @@ from force_bdss.core.input_slot_info import InputSlotInfo
 from force_bdss.core.data_value import DataValue
 from force_bdss.core.slot import Slot
 from force_bdss.tests import fixtures
+from force_bdss.tests.probe_classes.mco import ProbeMCOFactory
 
 try:
     import mock
@@ -223,7 +225,14 @@ class TestCoreEvaluationDriver(unittest.TestCase):
             output_slots_size=1,
             run_function=multiplier)
 
+        mco_factory = ProbeMCOFactory(self.plugin)
+        mco_model = mco_factory.create_model()
+        mco_model.kpis = [
+            KPISpecification(name="out1")
+        ]
+
         wf = Workflow(
+            mco=mco_model,
             execution_layers=[
                 ExecutionLayer(),
                 ExecutionLayer(),
@@ -281,7 +290,7 @@ class TestCoreEvaluationDriver(unittest.TestCase):
             InputSlotInfo(name="res2")
         ]
         model.output_slot_info = [
-            OutputSlotInfo(name="out1", is_kpi=True)
+            OutputSlotInfo(name="out1")
         ]
         wf.execution_layers[3].data_sources.append(model)