From 28fc0c6345b6e10b2e09dc49f01e28ebddd7edec Mon Sep 17 00:00:00 2001
From: Jon Burr <jon.burr@cern.ch>
Date: Mon, 23 Jan 2023 13:18:24 +0100
Subject: [PATCH] Track decisions IDs  on LinkInfo, not Decision

Track decisions IDs  on LinkInfo, not Decision
---
 .../Root/TrigCompositeUtilsRoot.cxx           |  4 +-
 .../TrigCompositeUtils/LinkInfo.h             | 26 ++++++++++-
 .../TrigCompositeUtils/TrigCompositeUtils.icc | 45 ++++++++++---------
 .../test/IPartCombItr_test.cxx                |  4 +-
 4 files changed, 53 insertions(+), 26 deletions(-)

diff --git a/Trigger/TrigSteer/TrigCompositeUtils/Root/TrigCompositeUtilsRoot.cxx b/Trigger/TrigSteer/TrigCompositeUtils/Root/TrigCompositeUtilsRoot.cxx
index 7403ae92829..d7e7562e185 100644
--- a/Trigger/TrigSteer/TrigCompositeUtils/Root/TrigCompositeUtilsRoot.cxx
+++ b/Trigger/TrigSteer/TrigCompositeUtils/Root/TrigCompositeUtilsRoot.cxx
@@ -113,7 +113,7 @@ namespace TrigCompositeUtils {
       }
     }
     return false;
-  }    
+  }
 
   bool passed( DecisionID id, const DecisionIDContainer& idSet ) {
     return idSet.count( id ) != 0;
@@ -731,7 +731,7 @@ namespace TrigCompositeUtils {
         HLT::Identifier legID = createLegName(chainName, legIdx);
         std::vector<LinkInfo<xAOD::IParticleContainer>> legFeatures;
         for (const LinkInfo<xAOD::IParticleContainer>& info : features)
-          if (isAnyIDPassing(info.source, {legID.numeric()}))
+          if (passed(legID.numeric(), info.decisions))
             legFeatures.push_back(info);
       combinations.addLeg(legMultiplicities.at(legIdx), std::move(legFeatures));
       }
diff --git a/Trigger/TrigSteer/TrigCompositeUtils/TrigCompositeUtils/LinkInfo.h b/Trigger/TrigSteer/TrigCompositeUtils/TrigCompositeUtils/LinkInfo.h
index 2469e20d9c8..2bf53ff6985 100644
--- a/Trigger/TrigSteer/TrigCompositeUtils/TrigCompositeUtils/LinkInfo.h
+++ b/Trigger/TrigSteer/TrigCompositeUtils/TrigCompositeUtils/LinkInfo.h
@@ -9,6 +9,8 @@
 #include "AthLinks/ElementLink.h"
 #include "AsgMessaging/StatusCode.h"
 
+#include <set>
+
 namespace TrigCompositeUtils {
   /**
    * @brief Additional information returned by the TrigerDecisionTool's feature retrieval, contained within the LinkInfo.
@@ -25,8 +27,18 @@ namespace TrigCompositeUtils {
   template<typename T>
   struct LinkInfo {
     LinkInfo() = default;
-    LinkInfo(const Decision* s, const ElementLink<T>& l, ActiveState as = ActiveState::UNSET)
-      : source{s}, link{l}, state{as} {}
+    LinkInfo(
+      const Decision* s, const ElementLink<T>& l, ActiveState as = ActiveState::UNSET)
+      : source{s}, link{l}, state{as} {
+        if (s)
+        {
+          decisions.insert(s->decisions().begin(), s->decisions().end());
+        }
+      }
+    
+    LinkInfo(
+      const Decision* s, const ElementLink<T>& l, ActiveState as, const DecisionIDContainer &decisions)
+      : source{s}, link{l}, state{as}, decisions(decisions) {}
 
     bool isValid() const {
       return source && link.isValid();
@@ -38,9 +50,19 @@ namespace TrigCompositeUtils {
       return (isValid() ? StatusCode::SUCCESS : StatusCode::FAILURE);
     }
 
+    /**
+     * @brief The node in the NavGraph for this feature
+     *
+     * Note that when retrieving features for multi-leg chains the same feature can be
+     * attached to multiple nodes and only one of those nodes will be returned here.
+    */
     const Decision* source{nullptr};
+    /// Link to the feature
     ElementLink<T> link;
+    /// Was the linked feature active for any requested chains
     ActiveState state{ActiveState::UNSET};
+    /// All decision IDs active for this feature
+    DecisionIDContainer decisions;
   };
 } //> end namespace TrigCompositeUtils
 
diff --git a/Trigger/TrigSteer/TrigCompositeUtils/TrigCompositeUtils/TrigCompositeUtils.icc b/Trigger/TrigSteer/TrigCompositeUtils/TrigCompositeUtils/TrigCompositeUtils.icc
index 7dc7b82489d..0e4994b6187 100644
--- a/Trigger/TrigSteer/TrigCompositeUtils/TrigCompositeUtils/TrigCompositeUtils.icc
+++ b/Trigger/TrigSteer/TrigCompositeUtils/TrigCompositeUtils/TrigCompositeUtils.icc
@@ -55,31 +55,31 @@ namespace TrigCompositeUtils {
       featureLinks.push_back(start->objectLink<T>(linkName));
     }
     const bool linkFound = (featureLinks.size() > 0);
+    const std::vector<DecisionID> &ids = decisionIDs(start);
     for (const ElementLink<T>& featureLink : featureLinks) {
       // Check for duplicates
-      if (std::none_of(links.begin(), links.end(), 
-        [&](const auto& li) { 
-          if (li.link == featureLink) {
-            if (li.source != start && linkName != initialRoIString() && linkName != roiString() && linkName != "l2cbroi" && linkName != viewString()) {
-              // Leaving this warning in for now, but there are known cases of the FS ROI being re-used
-              // in multiple outputs from the HLTSeeding. E.g. HLTNav_L1J:0 and HLTNav_L1MET:0
-              // so supressing for initialRoIString().
-              // Similar case for tau, just with an ROI created in Step1
-              const auto A = decisionToElementLink(start);
-              const auto B = decisionToElementLink(li.source);
-              ANA_MSG_WARNING ("Found '" << linkName << "' by multiple paths, but "
-                << "the links are coming from different places in the navigation graph. " << std::endl
-                << "From A:" << A.dataID() << ":" << A.index() << " Dump:" << std::endl << *start << std::endl
-                               << "From B:" << B.dataID() << ":" << B.index() << " Dump:" << std::endl << *(li.source));
-            }
-            return true;
+      bool duplicate = false;
+      for (LinkInfo<T> &info : links)
+      {
+        if (info.link == featureLink) {
+          if (info.source != start && linkName != initialRoIString() && linkName != roiString() && linkName != "l2cbroi" && linkName != viewString()) {
+            // Leaving this warning in for now, but there are known cases of the FS ROI being re-used
+            // in multiple outputs from the HLTSeeding. E.g. HLTNav_L1J:0 and HLTNav_L1MET:0
+            // so supressing for initialRoIString().
+            // Similar case for tau, just with an ROI created in Step1
+            const auto A = decisionToElementLink(start);
+            const auto B = decisionToElementLink(info.source);
+            ANA_MSG_WARNING ("Found '" << linkName << "' by multiple paths, but "
+              << "the links are coming from different places in the navigation graph. " << std::endl
+              << "From A:" << A.dataID() << ":" << A.index() << " Dump:" << std::endl << *start << std::endl
+                              << "From B:" << B.dataID() << ":" << B.index() << " Dump:" << std::endl << *(info.source));
           }
-          return false;
+          info.decisions.insert(ids.begin(), ids.end());
+          duplicate = true;
         }
-        ))
-      {
-        links.emplace_back(start, featureLink);
       }
+      if (!duplicate)
+        links.emplace_back(start, featureLink);
     }
     // Early exit
     if (linkFound && behaviour == TrigDefs::lastFeatureOfType) {
@@ -187,6 +187,7 @@ namespace TrigCompositeUtils {
 
  
     const Decision* decisionObj = navGraphNode->node();
+    const std::vector<DecisionID> &ids = decisionIDs(decisionObj);
     std::vector<ElementLink<CONTAINER>> featureLinks;
 
     // Look up what named links are available in the Decision Object
@@ -252,13 +253,15 @@ namespace TrigCompositeUtils {
       typename std::vector<LinkInfo<CONTAINER>>::iterator vecIt = std::find_if(features.begin(), features.end(), [&](const auto& li) { return li.link == featureLink; } );
       if (vecIt == features.end()) {
         // Link did not already exist - add it to the output
-        features.emplace_back( decisionObj, featureLink, state );
+        features.emplace_back( decisionObj, featureLink, state);
       } else {
         // Link already existed - if the link's state in the return vector is INACTIVE but is ACTIVE here,
         // then we need to change it to ACTIVE as this denotes one-or-more of the requested chains were active for the object.
         if (vecIt->state == ActiveState::INACTIVE and state == ActiveState::ACTIVE) {
           vecIt->state = ActiveState::ACTIVE;
         }
+        // Update the set of passed IDs
+        vecIt->decisions.insert(ids.begin(), ids.end());
       }
     }
 
diff --git a/Trigger/TrigSteer/TrigCompositeUtils/test/IPartCombItr_test.cxx b/Trigger/TrigSteer/TrigCompositeUtils/test/IPartCombItr_test.cxx
index 764a07a27ef..543f0943d02 100644
--- a/Trigger/TrigSteer/TrigCompositeUtils/test/IPartCombItr_test.cxx
+++ b/Trigger/TrigSteer/TrigCompositeUtils/test/IPartCombItr_test.cxx
@@ -28,7 +28,9 @@ namespace
     std::size_t index = idxBase++;
     return LinkInfo<xAOD::IParticleContainer>(
         owning.back().get(),
-        ElementLink<xAOD::IParticleContainer>(key, index));
+        ElementLink<xAOD::IParticleContainer>(key, index),
+        ActiveState::UNSET,
+        {});
   }
 
   /// Allow comparing link info objects
-- 
GitLab