Merge branch 'inputMaker_typelessFeatureRetrieval' into 'master'

[ATR-22206] Change InputMaker merging code to be typeless

See merge request atlas/athena!39204

Trigger/TrigSteer/DecisionHandling/DecisionHandling/InputMakerBase.h

@@ -47,8 +47,7 @@ This is a base class for HLT InputMakers to reduce boilerplate and enforce the c
   /// does the standard handling of input decisions: read from handles with all the checks, create merged output handles and link them, copies links and return outputHandles
   StatusCode decisionInputToOutput(const EventContext& context, SG::WriteHandle<TrigCompositeUtils::DecisionContainer>& outputHandle) const;
 
-  /// Checks for merge-able Decision objects coming from N upstream filters. Check based on stored element link in typed CONTAINER with 'linkNameToMatch'
-  template<typename CONTAINER>
+  /// Checks for merge-able Decision objects coming from N upstream filters. Check based on most-recent element link with name 'linkNameToMatch'. Works for any link type.
   size_t matchDecision(const TrigCompositeUtils::DecisionContainer* outDecisions, 
     const TrigCompositeUtils::Decision* toMatch, 
     const std::string& linkNameToMatch) const;
@@ -68,6 +67,4 @@ This is a base class for HLT InputMakers to reduce boilerplate and enforce the c
   
 };
 
-#include "InputMakerBase.icc"
-
 #endif // DECISIONHANDLING_INPUTMAKERBASE_H

Trigger/TrigSteer/DecisionHandling/DecisionHandling/InputMakerBase.icc

-/*
-  Copyright (C) 2002-2020 CERN for the benefit of the ATLAS collaboration
-*/
-
-template<typename CONTAINER>
-size_t InputMakerBase::matchDecision(const TrigCompositeUtils::DecisionContainer* outDecisions, 
-  const TrigCompositeUtils::Decision* toMatch, 
-  const std::string& linkNameToMatch) const
-{
-  const std::vector<TrigCompositeUtils::LinkInfo<CONTAINER>> myObject = TrigCompositeUtils::findLinks<CONTAINER>(toMatch, linkNameToMatch, TrigDefs::lastFeatureOfType);
-  
-  if (myObject.size() != 1) {
-    ATH_MSG_ERROR("InputMakerBase::matchDecision Did not locate exactly one object of type '" << ClassID_traits<CONTAINER>::typeName() << "' having searched for a link named '" << linkNameToMatch 
-      << "', found " << myObject.size() << ". Unable to match this Decision object.");
-    for (const auto& li : myObject) {
-      ATH_MSG_ERROR("  -- " << li.link.dataID() << ":" << li.link.index() << ". Dump:" << *(li.source)); 
-    }
-    return std::numeric_limits<std::size_t>::max();
-  }
-
-  // Look for match
-  for (size_t index = 0; index < outDecisions->size(); ++index) {
-    const TrigCompositeUtils::Decision* checkDecision = outDecisions->at(index);
-    if (checkDecision == nullptr) {
-      ATH_MSG_ERROR("Failed to get Decision object " << index << " of " << outDecisions->size());
-      return std::numeric_limits<std::size_t>::max();
-    }
-    const std::vector<TrigCompositeUtils::LinkInfo<CONTAINER>> checkObject = TrigCompositeUtils::findLinks<CONTAINER>(checkDecision, linkNameToMatch, TrigDefs::lastFeatureOfType);
-    if (checkObject.size() != 1) {
-      ATH_MSG_ERROR("Logic error. Expect myObject().size() == 1 and checkObject.size() == 1."
-        << " But have checkObject.size() = " << checkObject.size() << ". Unable to match this Decision object.");
-      return std::numeric_limits<std::size_t>::max();
-    }
-    if (myObject.at(0).link == checkObject.at(0).link) {
-      return index;
-    }
-  }
-
-  return std::numeric_limits<std::size_t>::max();
-}

Trigger/TrigSteer/DecisionHandling/src/InputMakerBase.cxx

@@ -94,13 +94,55 @@ StatusCode InputMakerBase::decisionInputToOutput(const EventContext& context, SG
 
 bool InputMakerBase::matchInCollection(const DecisionContainer* toMatchAgainst, const Decision* toMatch, size_t& matchIndex) const {
   if ( m_mergeUsingFeature ) {
-    matchIndex = matchDecision<xAOD::IParticleContainer>(toMatchAgainst, toMatch, featureString());
+    matchIndex = matchDecision(toMatchAgainst, toMatch, featureString());
   } else {
-    matchIndex = matchDecision<TrigRoiDescriptorCollection>(toMatchAgainst, toMatch, m_roisLink.value());
+    matchIndex = matchDecision(toMatchAgainst, toMatch, m_roisLink.value());
   }
   return (matchIndex != std::numeric_limits<std::size_t>::max());
 }
 
+size_t InputMakerBase::matchDecision(const DecisionContainer* outDecisions, const Decision* toMatch, const std::string& linkNameToMatch) const {
+  
+  std::set<const Decision*> cache; //!< Used to accelerate the recursive typelessFindLinks. Should be cleared between uses.
+  std::vector<uint32_t> keysA;
+  std::vector<uint32_t> clidsA;
+  std::vector<uint16_t> indiciesA;
+  TrigCompositeUtils::typelessFindLinks(toMatch, linkNameToMatch, keysA, clidsA, indiciesA, TrigDefs::lastFeatureOfType, &cache);
+  
+  if (keysA.size() != 1) {
+    ATH_MSG_ERROR("InputMakerBase::matchDecision Did not locate exactly one object having searched for a link named '" << linkNameToMatch 
+      << "', found " << keysA.size() << ". Unable to match this Decision object.");
+    for (size_t i = 0; i < keysA.size(); ++i) {
+      ATH_MSG_ERROR("  -- Key:" << keysA.at(i) << " Index:" << indiciesA.at(i) << " CLID:" << clidsA.at(i)); 
+    }
+    return std::numeric_limits<std::size_t>::max();
+  }
+
+  // Look for match
+  for (size_t index = 0; index < outDecisions->size(); ++index) {
+    const TrigCompositeUtils::Decision* checkDecision = outDecisions->at(index);
+    if (checkDecision == nullptr) {
+      ATH_MSG_ERROR("Failed to get Decision object " << index << " of " << outDecisions->size());
+      return std::numeric_limits<std::size_t>::max();
+    }
+    cache.clear();
+    std::vector<uint32_t> keysB;
+    std::vector<uint32_t> clidsB;
+    std::vector<uint16_t> indiciesB;
+    TrigCompositeUtils::typelessFindLinks(checkDecision, linkNameToMatch, keysB, clidsB, indiciesB, TrigDefs::lastFeatureOfType, &cache);
+    if (keysB.size() != 1) {
+      ATH_MSG_ERROR("Logic error. Expect toMatch size == 1 (confirmed) and checkObject size == 1."
+        << " But have checkObject size = " << keysB.size() << ". Unable to match this Decision object.");
+      return std::numeric_limits<std::size_t>::max();
+    }
+    if (keysA.at(0) == keysB.at(0) and clidsA.at(0) == clidsB.at(0) and indiciesA.at(0) == indiciesB.at(0)) {
+      return index;
+    }
+  }
+
+  return std::numeric_limits<std::size_t>::max();
+}
+
 
 void InputMakerBase::debugPrintOut(const EventContext& context, SG::WriteHandle<TrigCompositeUtils::DecisionContainer>& outputHandle) const{
   size_t validInput=0;

Trigger/TrigSteer/TrigCompositeUtils/Root/TrigCompositeUtilsRoot.cxx

@@ -351,24 +351,54 @@ namespace TrigCompositeUtils {
   {
     using namespace msgFindLink;
     if (visitedCache != nullptr) {
-      // We only need to recursivly explore back from each node in the graph once.
+      // We only need to recursively explore back from each node in the graph once.
       // We can keep a record of nodes which we have already explored, these we can safely skip over.
       if (visitedCache->count(start) == 1) {
         return false; // Early exit
       }
     }
-
+    // As the append vectors are user-supplied, perform some input validation. 
+    if (keyVec.size() != clidVec.size() or clidVec.size() != indexVec.size()) {
+      ANA_MSG_WARNING("In typelessFindLinks, keyVec, clidVec, indexVec must all be the same size. Instead have:"
+        << keyVec.size() << ", " << clidVec.size()  << ", " << indexVec.size());
+      return false;
+    }
+    // Locate named links. Both collections of links and individual links are supported.
     bool found = false;
+    std::vector<uint32_t> tmpKeyVec;
+    std::vector<uint32_t> tmpClidVec;
+    std::vector<uint16_t> tmpIndexVec;
     if (start->hasObjectCollectionLinks(linkName)) {
-      found = start->typelessGetObjectCollectionLinks(linkName, keyVec, clidVec, indexVec);
+      found = start->typelessGetObjectCollectionLinks(linkName, tmpKeyVec, tmpClidVec, tmpIndexVec);
     }
     if (start->hasObjectLink(linkName)) {
-      uint32_t key, clid;
-      uint16_t index;
-      found |= start->typelessGetObjectLink(linkName, key, clid, index);
-      keyVec.push_back(key);
-      clidVec.push_back(clid);
-      indexVec.push_back(index);
+      uint32_t tmpKey, tmpClid;
+      uint16_t tmpIndex;
+      found |= start->typelessGetObjectLink(linkName, tmpKey, tmpClid, tmpIndex);
+      tmpKeyVec.push_back(tmpKey);
+      tmpClidVec.push_back(tmpClid);
+      tmpIndexVec.push_back(tmpIndex);
+    }
+    // De-duplicate
+    for (size_t tmpi = 0; tmpi < tmpKeyVec.size(); ++tmpi) {
+      bool alreadyAdded = false;
+      const uint32_t tmpKey = tmpKeyVec.at(tmpi);
+      const uint32_t tmpClid = tmpClidVec.at(tmpi);
+      const uint16_t tmpIndex = tmpIndexVec.at(tmpi);
+      for (size_t veci = 0; veci < keyVec.size(); ++veci) {
+        if (keyVec.at(veci) == tmpKey 
+          and clidVec.at(veci) == tmpClid
+          and indexVec.at(veci) == tmpIndex)
+        {
+          alreadyAdded = true;
+          break;
+        }
+      }
+      if (!alreadyAdded) {
+        keyVec.push_back( tmpKey );
+        clidVec.push_back( tmpClid );
+        indexVec.push_back( tmpIndex );
+      }
     }
     // Early exit
     if (found && behaviour == TrigDefs::lastFeatureOfType) {
@@ -395,7 +425,7 @@ namespace TrigCompositeUtils {
     // only want the most recent.
     // Hence we can supply TrigDefs::lastFeatureOfType.                                                         /--> parent3(link)
     // We can still have more then one link found if there is a branch in the navigation. E.g. start --> parent1 --> parent2(link)
-    // If both parent2 and parent3 posessed an admisable ElementLink, then the warning below will trigger, and only one of the
+    // If both parent2 and parent3 possessed an admissible ElementLink, then the warning below will trigger, and only one of the
     // links will be returned (whichever of parent2 or parent3 happened to be the first seed of parent1).
     std::vector<uint32_t> keyVec;
     std::vector<uint32_t> clidVec;

Trigger/TrigSteer/TrigCompositeUtils/TrigCompositeUtils/TrigCompositeUtils.h

@@ -457,7 +457,7 @@ namespace TrigCompositeUtils {
                           branch once a link has been located and collected. 
    * @param[inout] visitedCache Optional cache used by the recursive algorithm to avoid exploring each node multiple times. 
    */
-  bool typelessfindLinks(const Decision* start, 
+  bool typelessFindLinks(const Decision* start, 
     const std::string& linkName,
     std::vector<uint32_t>& key,
     std::vector<uint32_t>& clid,