Merge branch 'apearce-moore-51' into 'master'

Declare data dependencies in ThOr functors

See merge request !2343

Phys/FunctorCore/CMakeLists.txt

@@ -38,5 +38,7 @@ gaudi_add_unit_test(TestFunctors
                     TYPE Boost)
 
 gaudi_install_python_modules()
-gaudi_add_test(python COMMAND nosetests --with-doctest -v ${CMAKE_CURRENT_SOURCE_DIR}/python)
+gaudi_add_test(python
+               COMMAND python -m pytest -v --doctest-modules
+                       ${CMAKE_CURRENT_SOURCE_DIR}/python)
 gaudi_add_test(QMTest QMTEST)

Phys/FunctorCore/python/Functors/__init__.py

@@ -9,6 +9,7 @@
 # or submit itself to any jurisdiction.                                       #
 ###############################################################################
 from Functors.grammar import Functor, BoundFunctor
+from PyConf.dataflow import DataHandle
 FILTER = Functor(
     'FILTER',
     'Filter',
@@ -56,7 +57,8 @@ MINIP = Functor(
     "TrackLike.h",
     """Calculate the minimum impact parameter w.r.t. any of the given vertices.
     MINIPCUT may be more efficient.""",
-    Params=[('Vertices', 'TES location of input [primary] vertices', str)],
+    Params=[('Vertices', 'TES location of input [primary] vertices',
+             DataHandle)],
     TemplateParams=[('VerticesType', 'Input vertex container type')])
 MINIPCHI2 = Functor(
     'MINIPCHI2',
@@ -64,7 +66,8 @@ MINIPCHI2 = Functor(
     "TrackLike.h",
     """Calculate the minimum impact parameter chi2 w.r.t. any of the given
     vertices. MINIPCHI2CUT may be more efficient.""",
-    Params=[('Vertices', 'TES location of input [primary] vertices', str)],
+    Params=[('Vertices', 'TES location of input [primary] vertices',
+             DataHandle)],
     TemplateParams=[('VerticesType', 'Input vertex container type')])
 MINIPCUT = Functor(
     'MINIPCUT',
@@ -73,7 +76,8 @@ MINIPCUT = Functor(
     """Require the minimum impact parameter w.r.t. any of the given vertices is
     greater than some threshold.""",
     Params=[('IPCut', 'The impact parameter cut value', float),
-            ('Vertices', 'TES location of input [primary] vertices', str)],
+            ('Vertices', 'TES location of input [primary] vertices',
+             DataHandle)],
     TemplateParams=[('VerticesType', 'Input vertex container type')])
 MINIPCHI2CUT = Functor(
     'MINIPCHI2CUT',
@@ -82,7 +86,8 @@ MINIPCHI2CUT = Functor(
     """Require the minimum impact parameter chi2 w.r.t. any of the given
     vertices is greater than some threshold.""",
     Params=[('IPChi2Cut', 'The impact parameter chi2 cut value', float),
-            ('Vertices', 'TES location of input [primary] vertices', str)],
+            ('Vertices', 'TES location of input [primary] vertices',
+             DataHandle)],
     TemplateParams=[('VerticesType', 'Input vertex container type')])
 ALL = Functor('ALL', "AcceptAll", "Function.h",
               "Accept everything; always evaluates to 'true'.")
@@ -177,7 +182,8 @@ SIZE = Functor(
     "TES.h",
     "Get the size of the given container",
     Params=[('Container',
-             'The TES location of the container whose size we return', str)],
+             'The TES location of the container whose size we return',
+             DataHandle)],
     TemplateParams=[('ContainerType', 'Type of the container')])
 BPVETA = Functor(
     'BPVETA',
@@ -185,7 +191,8 @@ BPVETA = Functor(
     'Composite.h',
     '''Compute the pseudorapidity of the vector connecting the associated
     [primary] vertex to the composite particle decay vertex.''',
-    Params=[('Vertices', 'TES location of input [primary] vertices.', str)],
+    Params=[('Vertices', 'TES location of input [primary] vertices.',
+             DataHandle)],
     TemplateParams=[('VerticesType', 'Input vertex container type')])
 BPVCORRM = Functor(
     'BPVCORRM',
@@ -195,7 +202,8 @@ BPVCORRM = Functor(
     hypotheses and the associated [primary] vertex.''',
     Params=[('Mass', 'Mass hypotheses to apply to the composite children',
              float),
-            ('Vertices', 'TES location of input [primary] vertices.', str)],
+            ('Vertices', 'TES location of input [primary] vertices.',
+             DataHandle)],
     TemplateParams=[('VerticesType', 'Input vertex container type')])
 BPVDIRA = Functor(
     'BPVDIRA',
@@ -204,7 +212,8 @@ BPVDIRA = Functor(
     '''Compute the cosine of the angle between the particle momentum and the
     vector from the associated [primary] vertex to the composite particle
     decay vertex.''',
-    Params=[('Vertices', 'TES location of input [primary] vertices.', str)],
+    Params=[('Vertices', 'TES location of input [primary] vertices.',
+             DataHandle)],
     TemplateParams=[('VerticesType', 'Input vertex container type')])
 BPVIPCHI2 = Functor(
     'BPVIPCHI2',
@@ -213,7 +222,8 @@ BPVIPCHI2 = Functor(
     '''Return the impact parameter chi2 w.r.t. the associated vertex, assuming
     that it is from the container of vertices that is passed. If no association
     is available, compute one.''',
-    Params=[('Vertices', 'TES location of input [primary] vertices.', str)],
+    Params=[('Vertices', 'TES location of input [primary] vertices.',
+             DataHandle)],
     TemplateParams=[('VerticesType', 'Input vertex container type')])
 BPVFDCHI2 = Functor(
     'BPVFDCHI2',
@@ -222,7 +232,8 @@ BPVFDCHI2 = Functor(
     '''Return the flight distance chi2 w.r.t. the associated vertex, assuming
     that it is from the container of vertices that is passed. If no association
     is available, compute one.''',
-    Params=[('Vertices', 'TES location of input [primary] vertices.', str)],
+    Params=[('Vertices', 'TES location of input [primary] vertices.',
+             DataHandle)],
     TemplateParams=[('VerticesType', 'Input vertex container type')])
 BPVLTIME = Functor(
     'BPVLTIME',
@@ -231,28 +242,29 @@ BPVLTIME = Functor(
     '''Return the particle lifetime w.r.t. the associated vertex, assuming
     that it is from the container of vertices that is passed. If no association
     is available, compute one.''',
-    Params=[('Vertices', 'TES location of input [primary] vertices.', str)],
+    Params=[('Vertices', 'TES location of input [primary] vertices.',
+             DataHandle)],
     TemplateParams=[('VerticesType', 'Input vertex container type')])
 RUNNUMBER = Functor(
     'RUNNUMBER',
     'TES::RunNumber',
     'TES.h',
     '''Return the run number from ODIN.''',
-    Params=[('ODIN', 'TES location of ODIN information.', str)],
+    Params=[('ODIN', 'TES location of ODIN information.', DataHandle)],
     TemplateParams=[('ODINType', 'Type of the ODIN object')])
 EVENTNUMBER = Functor(
     'EVENTNUMBER',
     'TES::EventNumber',
     'TES.h',
     '''Return the event number from ODIN.''',
-    Params=[('ODIN', 'TES location of ODIN information.', str)],
+    Params=[('ODIN', 'TES location of ODIN information.', DataHandle)],
     TemplateParams=[('ODINType', 'Type of the ODIN object')])
 EVENTTYPE = Functor(
     'EVENTTYPE',
     'TES::EventType',
     'TES.h',
     '''Return the event type from ODIN.''',
-    Params=[('ODIN', 'TES location of ODIN information.', str)],
+    Params=[('ODIN', 'TES location of ODIN information.', DataHandle)],
     TemplateParams=[('ODINType', 'Type of the ODIN object')])
 
 # Examples:
@@ -271,15 +283,18 @@ Ex_TBL = Functor('TBL', 'Examples::ThorBeatsLoki', 'Example.h', ''' ... ''')
 
 def mva_input_formatter(config):
     from Functors.grammar import python_to_cpp_str
-    inputs = []
+    mva_inputs = []
     headers = []
+    # Data dependencies
+    inputs = []
     for key, val in list(config.items()):
         assert type(key) == str
         key_code, key_headers = python_to_cpp_str(key)
         val_code, val_headers = python_to_cpp_str(val)
-        inputs.append('MVAInput( ' + key_code + ', ' + val_code + ' )')
+        mva_inputs.append('MVAInput( ' + key_code + ', ' + val_code + ' )')
         headers += key_headers + val_headers
-    return ', '.join(inputs), list(set(headers))
+        inputs += val.data_dependencies()
+    return ', '.join(mva_inputs), list(set(headers)), inputs
 
 
 def mva_impl_formatter(config):
@@ -298,13 +313,18 @@ MVA = Functor(
 
 
 def comb_locations_formatter(locations):
-    from Functors.grammar import python_to_cpp_str
-    expressions, headers = [], []
+    from Functors.grammar import python_to_cpp_str, value_matches_type
+    expressions, headers, inputs = [], [], []
     for loc in locations:
+        # The functor framework is unable to introspect type requirements of
+        # composites, e.g. 'list-of-DataHandle', only 'list'. We enforce the
+        # list-of-DataHandle type requirement of the COMB functor here instead
+        assert value_matches_type(loc, DataHandle)
         code, headers = python_to_cpp_str(loc)
         expressions.append(code)
         headers += headers
-    return ', '.join(expressions), list(set(headers))
+        inputs.append(loc)
+    return ', '.join(expressions), list(set(headers)), inputs
 
 
 def comb_types_formatter(type_list):

Phys/FunctorCore/python/Functors/grammar.py

@@ -14,10 +14,25 @@ Pure-python classes used as building blocks of functor expressions.
 from builtins import zip
 from builtins import object
 import json
+import logging
 from GaudiKernel.Configurable import Configurable
 from Functors.common import header_prefix, top_level_namespace
+from PyConf.dataflow import DataHandle
 from itertools import chain
 
+log = logging.getLogger(__name__)
+
+
+def value_matches_type(value, expected_type):
+    # Special case for DataHandle given as str: we allow it, but warn
+    if expected_type is DataHandle and isinstance(value, str):
+        log.warning(("String '%s' given as substitute for DataHandle; "
+                     "PyConf will not be able to infer this data dependency"),
+                    value)
+        return True
+    # For everything else, types must match exactly
+    return isinstance(value, expected_type)
+
 
 def bind_if_needed(functor):
     """Return a BoundFunctor constructed from the argument.
@@ -37,6 +52,11 @@ def bind_if_needed(functor):
 
 
 def python_to_cpp_str(obj):
+    # Reduce complex Python types down to simple Python representations
+    if isinstance(obj, DataHandle):
+        obj = obj.location
+
+    # Convert simple Python types to C++ representations
     if type(obj) == int:
         return 'std::integral_constant<int, {}>{{}}'.format(obj), []
     elif type(obj) == float:
@@ -194,12 +214,37 @@ class BoundFunctor(FunctorBase, type(
     {})):  #hack to ensure that the BoundFunctor is parsed correctly
     """A functor that has had all its parameters fully specified."""
 
-    def __init__(self, code=None, nice_code=None, headers=[], nice_name=None):
+    def __init__(self,
+                 code=None,
+                 nice_code=None,
+                 headers=[],
+                 inputs=None,
+                 nice_name=None):
         self._strrep = nice_code  # RUNNUMBER(ODIN=...)
         self._strname = nice_name  # RUNNUMBER
         self._code = code
+        # Unique DataHandle objects held by this functor
+        self._inputs = list(set(inputs or []))
         self._headers = headers
 
+    def __getstate__(self):
+        """Return serialisation state for pickling.
+
+        Note:
+            The pickled state **discards explicit DataHandle data
+            dependencies**. This is because these can reference to a
+            PyConf.Algorithm object, and PyConf make no guarantees about
+            their pickle-ability.
+
+            The implicit data dependency is retained, as encoded in the TES
+            location of the DataHandle in the string encoding of the functor.
+        """
+        return (self._strrep, self._strname, self._code, [], self._headers)
+
+    def __setstate__(self, state):
+        """Load serialisation state from pickled representation."""
+        self._strrep, self._strname, self._code, self._inputs, self._headers = state
+
     def code(self):
         return self._code
 
@@ -213,6 +258,14 @@ class BoundFunctor(FunctorBase, type(
         assert not any([len(h) == 0 for h in self._headers])
         return self._headers
 
+    def data_dependencies(self):
+        """Return a list of DataHandle inputs to this functor.
+
+        This implements the API which allows PyConf to deduce that a functor
+        property must 'inject' dependencies into its owning Algorithm.
+        """
+        return self._inputs
+
     def __str__(self):
         return str((self.code(), self.headers(), self.code_repr()))
 
@@ -250,7 +303,8 @@ class ComposedBoundFunctor(BoundFunctor):
             self,
             code=code,
             nice_code=nice_code,
-            headers=list(set(func1.headers() + func2.headers())))
+            headers=list(set(func1.headers() + func2.headers())),
+            inputs=func1.data_dependencies() + func2.data_dependencies())
 
 
 class InvertedBoundFunctor(BoundFunctor):
@@ -265,7 +319,8 @@ class InvertedBoundFunctor(BoundFunctor):
             self,
             code='~( {f1} )'.format(f1=bound_functor.code()),
             nice_code='~{f1}'.format(f1=bound_functor.code_repr()),
-            headers=bound_functor.headers())
+            headers=bound_functor.headers(),
+            inputs=bound_functor.data_dependencies())
 
 
 class WrappedBoundFunctor(BoundFunctor):
@@ -279,8 +334,10 @@ class WrappedBoundFunctor(BoundFunctor):
                  *arguments):
         bound_functors = [bind_if_needed(arg) for arg in arguments]
         headers = [wrapping_function_header]
+        inputs = []
         for bound_functor in bound_functors:
             headers += bound_functor.headers()
+            inputs += bound_functor.data_dependencies()
         code_arguments = ', '.join(
             [bound_functor.code() for bound_functor in bound_functors])
         code = '{wrap}( {args} )'.format(
@@ -292,7 +349,11 @@ class WrappedBoundFunctor(BoundFunctor):
         headers = list(set(headers))
 
         BoundFunctor.__init__(
-            self, code=code, nice_code=nice_code, headers=headers)
+            self,
+            code=code,
+            nice_code=nice_code,
+            headers=headers,
+            inputs=inputs)
 
 
 def Functor(representation,
@@ -344,6 +405,8 @@ def Functor(representation,
         cpp_headers = [self._header]
         cpp_arguments = []
         repr_arguments = []
+        # Input DataHandle dependencies
+        inputs = []
         for argtuple in self._arguments:
             argname, argdocs, argtype = argtuple[:3]
             formatter = argtuple[3] if len(argtuple) > 3 else None
@@ -359,13 +422,16 @@ def Functor(representation,
                     # functor that doesn't expect arguments (e.g. PT)...
                     provided_value = provided_value()
                     value_repr = provided_value.code_repr()
-                if isinstance(provided_value, argtype):
+                    # Propagate dependencies from the property to self
+                    inputs += provided_value.data_dependencies()
+                if value_matches_type(provided_value, argtype):
                     # all good
                     repr_arguments.append((argname, value_repr))
                     if formatter is not None:
-                        code, headers = formatter(provided_value)
+                        code, headers, value_inputs = formatter(provided_value)
                         cpp_arguments.append(doc_comment + code)
                         cpp_headers += headers
+                        inputs += value_inputs
                         #TODO also format the repr
                     else:
                         try:
@@ -378,6 +444,8 @@ def Functor(representation,
                                 .format(
                                     arg=provided_value,
                                     type=type(provided_value)))
+                    if isinstance(provided_value, DataHandle):
+                        inputs.append(provided_value)
                 else:
                     raise Exception(
                         "Incompatible type {provided} given, expected {expected}"
@@ -438,6 +506,7 @@ def Functor(representation,
             code=cpp_str,
             nice_code=repr_str,
             headers=list(set(cpp_headers)),
+            inputs=inputs,
             nice_name=representation)
 
     call_doc_lines = ["Keyword arguments:"]

Phys/FunctorCore/python/Functors/tests/test_grammar.py

@@ -12,9 +12,11 @@
 from __future__ import print_function
 from __future__ import division
 from past.utils import old_div
+import pickle
 from Functors import PT, ISMUON, MINIPCUT
 from Functors import math as fmath
 from GaudiKernel.SystemOfUnits import *
+from PyConf.Algorithms import Gaudi__Examples__IntDataProducer
 
 
 def not_empty(f):
@@ -79,3 +81,25 @@ def test_disabled_logical_operators():
         pass
     else:
         raise Exception("Use of logical `not` did not raise an exception.")
+
+
+def test_serialisation():
+    """A BoundFunctor must be seriasable by the pickle module."""
+    functor = MINIPCUT(IPCut=1.0, Vertices='Test/Path')
+    # Use the same protocol as in `gaudirun.py -o options.pkl`
+    loaded = pickle.loads(pickle.dumps(functor, protocol=-1))
+    assert functor.code() == loaded.code()
+    assert functor.code_repr() == loaded.code_repr()
+    assert functor.headers() == loaded.headers()
+    assert functor.name() == loaded.name()
+    assert repr(functor) == repr(loaded)
+    assert str(functor) == str(loaded)
+
+    # Should also work with DataHandle inputs
+    pvs = Gaudi__Examples__IntDataProducer().OutputLocation
+    functor = MINIPCUT(IPCut=1.0, Vertices=pvs)
+    loaded = pickle.loads(pickle.dumps(functor, protocol=-1))
+    # It is expected that the data dependencies are lost during serialisation;
+    # see BoundFunctor.__getstate__
+    assert len(functor.data_dependencies()) > len(loaded.data_dependencies())
+    assert len(loaded.data_dependencies()) == 0

Phys/FunctorCore/python/Functors/tests/test_pyconf.py

+###############################################################################
+# (c) Copyright 2021 CERN for the benefit of the LHCb Collaboration           #
+#                                                                             #
+# This software is distributed under the terms of the GNU General Public      #
+# Licence version 3 (GPL Version 3), copied verbatim in the file "COPYING".   #
+#                                                                             #
+# In applying this licence, CERN does not waive the privileges and immunities #
+# granted to it by virtue of its status as an Intergovernmental Organization  #
+# or submit itself to any jurisdiction.                                       #
+###############################################################################
+from Functors import BPVFDCHI2, CHI2DOF, COMB, MINIPCUT, MVA, SUM
+from Functors.math import log
+from PyConf.Algorithms import FunctorExampleAlg, Gaudi__Examples__IntDataProducer, Gaudi__Examples__VectorDataProducer
+from PyConf.dataflow import dataflow_config
+
+
+def test_pyconf_integration():
+    """PyConf should be able to deduce inputs of functor-holding algorithms.
+
+    Data dependencies of a functor should be inherited by a PyConf.Algorithm
+    that holds that functor.
+    """
+    # Pretend that we can load some PVs from an input file
+    # Note that Python functor representations do not perform type-checking of
+    # their data dependencies (it would be very difficult to implement, given
+    # the templated nature of most functors)
+    pvs = Gaudi__Examples__IntDataProducer().OutputLocation
+    pvs2 = Gaudi__Examples__VectorDataProducer().OutputLocation
+
+    # PyConf asks Algorithm properties for inputs to inject using the
+    # `data_dependencies()` method, so check that works
+    functor = MINIPCUT(IPCut=1.0, Vertices=pvs)
+    functor2 = MINIPCUT(IPCut=2.0, Vertices=pvs2)
+    assert len(functor.data_dependencies()) == 1
+    # Should be propagated by composition and operations
+    assert len((functor & functor2).data_dependencies()) == 2
+    assert len((functor | functor2).data_dependencies()) == 2
+    assert len((~functor).data_dependencies()) == 1
+    assert len(log(functor).data_dependencies()) == 1
+
+    # A more complex functor
+    mva = MVA(
+        MVAType="MatrixNet",
+        Config={"MatrixnetFile": ""},
+        Inputs={
+            "chi2": CHI2DOF,
+            "sumfdchi2": BPVFDCHI2(pvs) + BPVFDCHI2(pvs2),
+        },
+    )
+    assert len(mva.data_dependencies()) == 2
+
+    # Another complex functor, including a list of data dependencies
+    # (`ChildContainers`) and a functor which takes bound functor which has
+    # dependencies
+    comb = COMB(
+        Functor=SUM(BPVFDCHI2(pvs)),
+        ChildContainers=[pvs2],
+        # Not actually instantiating the C++ functor, so can give dummy types
+        ChildContainerTypes=[('', '')])
+    print(comb.data_dependencies())
+    assert len(comb.data_dependencies()) == 2
+
+    alg = FunctorExampleAlg(Cut=functor)
+    assert len(alg.inputs) == 1
+    assert next(iter(alg.inputs.values()))[0] is pvs
+
+
+def test_configurable_instantiation():
+    """PyConf should be able to instantiate a functor-holding configurable."""
+    pvs = Gaudi__Examples__IntDataProducer().OutputLocation
+    functor = MINIPCUT(IPCut=1.0, Vertices=pvs)
+    alg = FunctorExampleAlg(Cut=functor)
+
+    # Convert PyConf Algorithm to Gaudi Configurable
+    c = dataflow_config()
+    c.update(alg.configuration())
+    c.apply()
+
+
+def test_string_datahandle_warning(caplog):
+    """Passing a str for a DataHandle should print a warning."""
+    pvs = Gaudi__Examples__IntDataProducer().OutputLocation
+
+    # No warning
+    MINIPCUT(IPCut=1.0, Vertices=pvs)
+    assert len(caplog.records) == 0
+
+    # Use the DataHandle's concrete string location, get a warning
+    MINIPCUT(IPCut=1.0, Vertices=pvs.location)
+    assert len(caplog.records) == 1
+    record, = caplog.records
+    assert record.levelname == "WARNING", record
+    assert "not be able to infer this data dependency" in record.msg, record