Skip to content
Snippets Groups Projects

Draft: New adapter (F.EVENT_INFO) to store trigger/event-level with FunTuple

Closed Draft: New adapter (F.EVENT_INFO) to store trigger/event-level with FunTuple
AM_eventinfo into master
4 unresolved threads
Closed Abhijit Mathad requested to merge AM_eventinfo into master
4 unresolved threads

The MR adds a new adapter F.EVENT_INFO that allows to store event-level and trigger information in FunTuple.

Motivation for this is that, FunTuple is a candidate-by-candidate based tupling algorithm and therefore only takes as input functors that act on the candidate. To store event-level information, base functors will have data dependency on objects not related to the candidate e.g. LHCb::ODIN, DecReports (TISTOS is a separate beast). This adapter solve the issue by adding a a fake candidate dependency on the event information. As a result the adapter takes a data dependent functor and outputs the result, completely ignoring the input candidate. Addresses the issue: #285 (closed).

The user code in FunTuple looks like:

#define event info

evt_vars = {
    "EVENTNUMBER": F.EVENT_INFO(F.EVENTNUMBER, ODIN=odin),
    "RUNNUMBER": F.EVENT_INFO(F.RUNNUMBER, ODIN=odin)
}

PS: The composition of functors was discussed in the issue (see comment), however on the python side the event info functors are themselves defined to be DataDepWrapper, which I think will have to be changed for composition to work. Additionally, I am not sure when the composition MR will be in. So have opted for an option that requires least changes to the current framework, with no additional dependencies.

To be tested with: Analysis!867 (merged) and DaVinci!654 (merged)

FYI: @graven , @sstahl , @pkoppenb , @erodrigu , @ldufour , @gligorov

Edited by Abhijit Mathad

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
Phys/FunctorCore/include/Functors/TES.h
108
109 CandidateToEventWrapper( detail::DataDepWrapper<Function, F, ODIN> f ) : m_f{std::move( f )} {}
110
111 /// Bind the functor to top_level algorithm that owns the functor (FunTuple of selection in sprucing).
112 void bind( TopLevelInfo& top_level ) { detail::bind( m_f, top_level ); }
113
114 /// Prepare function that outputs another function that takes
115 /// as input data (e.g. LHCb::Odin) and candidate. The candidate is simply ignored.
116 auto prepare( EventContext const& evtCtx, TopLevelInfo const& top_level ) const {
117 return
118 [fun = detail::prepare( m_f, evtCtx, top_level )]( [[maybe_unused]] auto const& particle ) { return fun(); };
119 }
120
121 private:
122 detail::DataDepWrapper<Function, F, ODIN> m_f;
123 };
  • Comment on lines +106 to +123
    Christoph Hasse
    Christoph Hasse @chasse ·
    Contributor

    I think for what this functor wants to do, the implementation is quite constraint and thus would make it hard to reuse somewhere else.
    If I understand correctly, given a functor F1 that is fully specified, you would like to create a Functor F2 which given any input, will return F1().

    So I'd give it a more general name -> IgnoreInputWrapper? or anything else that reflects better what it is the Functor does, not how it will be used.

    I think it's then enough to only constrain the input argument to be of type functor. And I'd make the returned lambda a generic lambda ->

              [fun = detail::prepare( m_f, evtCtx, top_level )]( auto&&... ) { return fun(); };
  • Abhijit Mathad
    Abhijit Mathad @amathad ·
    Author Developer

    FWIW, I agree here with you regarding name choice.

  • Please register or sign in to reply
  • Christoph Hasse
    Christoph Hasse @chasse started a thread on the diff
    • Phys/FunctorCore/python/Functors/__init__.py
    524 524 return MAP_INPUT_(
    525 525 Functor=Functor, Relations=Relations, RelationsType=Relations.type)
    526
    527
    528 EVENT_INFO_ = Functor(
    529 'EVENT_INFO',
    530 'TES::CandidateToEventWrapper',
    531 'TES.h',
    532 '''Functor designed for FunTuple to get event level information.''',
    533 Params=[('Functor', "The functor to apply to the data object.",
    534 BoundFunctor)],
    535 TemplateParams=[('DataObjType',
    536 'A string specifying the data object type')])
    537
    538
    539 def EVENT_INFO(Functor: BoundFunctor, **base_func_args) -> BoundFunctor:
  • Christoph Hasse
    Christoph Hasse @chasse started a thread on the diff
    • Phys/FunctorCore/python/Functors/__init__.py
    545
    546 Returns:
    547 The result of invoking
    548
    549 `EVENT_INFO_(Functor=Functor(**base_func_args), DataObjType=base_func_args[datatype])`
    550 """
    551 datatypes = [k for k in base_func_args.keys() if 'Type' in k]
    552 if len(datatypes) == 0:
    553 return EVENT_INFO_(Functor=Functor(**base_func_args))
    554 else:
    555 assert len(
    556 datatypes
    557 ) == 1, "Too many data types passed, this functor only supports one."
    558 return EVENT_INFO_(
    559 Functor=Functor(**base_func_args),
    560 DataObjType=base_func_args[datatypes[0]])
    • Comment on lines +551 to +560
      Christoph Hasse
      Christoph Hasse @chasse ·
      Contributor

      This should probably be tests as well.
      But I'm having a bit of a hard time figuring out what exactly is happening with the base_func_args... :thinking:
      Is that just to e.g. the LHCb::Odin out of the arguments of e.g. the RunNumber functor?

      If we make CandidateToEventWrapper more general then I guess all of this could go away, right?

    • Abhijit Mathad
      Abhijit Mathad @amathad ·
      Author Developer

      Exactly the EVENT_INFO "inherits" the arguments of RunNumber. One would have to still do Functor=Functor(**base_func_args) though so not sure if everything should go away.

      Edited by Abhijit Mathad
    • Christoph Hasse
      Christoph Hasse @chasse ·
      Contributor

      @amathad could we with the above proposal not simply do:

      FunTuple_RUNNUMBER = IgnoreInputWrapper(RUNNUMBER("path/to/odin"))

      That is IgnoreInputWrapper simply takes a BoundFunctor similar to e.g. MAP_INPUT

      Edited by Christoph Hasse
    • Abhijit Mathad
      Abhijit Mathad @amathad ·
      Author Developer

      True, but I wanted the user interface to be: F.EVENT_INFO(F.RUNNUMBER, ODIN) rather than F.EVENT_INFO(F.RUNNUMBER(ODIN)), perhaps my first option introduces confusion to users if they look up the definition of F.RUNNUMBER that has data dependency.

    • Please register or sign in to reply
  • Christoph Hasse
    Christoph Hasse @chasse started a thread on the diff
    • Phys/FunctorCore/include/Functors/TES.h
    88 88 return odin.eventType();
    89 89 }
    90 90 };
    91
    92 /** @class Functors::detail::CandidateToEventWrapper
    93 * @brief Class designed ONLY for FunTuple, "candidate-by-candidate" based tupling algorithm, as opposed to
    94 * "event-by-event" based, to store event-level information. It introduces a fake candidate dependency on the event
    95 * information.
    96 *
    97 * Class to be used only in FunTuple, which is a candidate-by-candidate based
    98 * tupling algorithm and therefore only takes as input functors that act on the candidate.
    99 * To store event-level information, we introduce a fake candidate dependency on the event information.
    100 * As a result this adapter takes a data dependent functor (e.g. the data dependency could be on LHCb::Odin)
    101 * gets the result, completely ignoring the input candidate.
    • Comment on lines +97 to +101
      Christoph Hasse
      Christoph Hasse @chasse ·
      Contributor

      I've read #285 (closed), and maybe a Zoom chat with everyone just to hash out this discussion is worth it? WDYT @amathad ?

      I think that the here proposed solution is quite clever and would probably work in terms of getting Event info into the tuple.
      What isn't clear to me in this discussion is how pragmatic we already need to be about the implementation.
      After all this is code that will be somewhat user facing. While the basic use case will only include the python functors, the C++ side of those functors is the first level of code that people will have to understand and dig through whenever there is a bug or behavior isn't documented well enough or needs extending.

      So I think @ldufour raises very good points that we need to think about how the user will interact with that code.
      With that in mind,
      On the first level: I think CANDIDATE_RUN_NUMBER or CandidateToEventWrapper leaks an implementation detail to the user. IMHO, it's never going to be intuitive to the user that the event level info like RunNumber has something to do with a candidate.
      Even having to wrap RunNumber in a EventInfo wrapper in some but not all cases is probably a hurdle....
      And on C++ level: I am worried about how many levels of abstraction are already stacked on top of each other to achieve something as simple as "get the RunNumber"...

      Edited by Christoph Hasse
    • Sascha Stahl
      Sascha Stahl @sstahl ·
      Developer

      Have you thought about adding an additional functore desc to FunTuple.cpp to avoid having to use CandidateToEventWrapper? The functor desc would have a different signature, in this case probably void as all data dependencies are handled by DataHandles.

      It would be similar to the implementation in https://gitlab.cern.ch/lhcb/Rec/-/blob/master/Phys/SelAlgorithms/include/SelAlgorithms/DumpContainer.h#L68 .

    • Abhijit Mathad
      Abhijit Mathad @amathad ·
      Author Developer

      @chasse : Fully agree and the discussion there was with regard to a "hacky" way to get event info in FunTuple. I followed the path of least action and hence the MR.

      Alternate would be to have an additional functordesc in FunTuple. Have to check how trivial/non-trivial that could be and perhaps pursue that...(this week's a little busy but will get back).

      Edited by Abhijit Mathad
    • Please register or sign in to reply
  • Patrick Koppenburg added lhcb-run3-cleanup label

    added lhcb-run3-cleanup label

  • L
    Software for LHCb @lhcbsoft ·
    Developer
  • Abhijit Mathad
    Abhijit Mathad @amathad ·
    Author Developer

    I have added support for void functors in FunTuple (Analysis!867 (merged)). So closing this MR in favour of a new one (!2747 (merged)).

  • Abhijit Mathad mentioned in issue #294

    mentioned in issue #294

  • Abhijit Mathad closed

    closed

    • Please register or sign in to reply