Update after review

- change name of CounterROS::newEvent parameter
- retrieve tc details outside loop
- move filling counting histograms
- change monitored time to milisec
- use find instead of sort to not mix indicies

Trigger/TrigCost/TrigCostAnalysis/CMakeLists.txt

@@ -9,7 +9,7 @@ find_package( ROOT COMPONENTS Core Hist RIO )
 # Athena algorithm to do cost analysis and produce histograms
 atlas_add_component( TrigCostAnalysis
                      src/*.cxx src/monitors/*.cxx src/counters/*.cxx src/components/TrigCostAnalysis_entries.cxx
-                     PRIVATE_LINK_LIBRARIES ${ROOT_LIBRARIES} GaudiKernel AthAnalysisBaseCompsLib TrigDecisionToolLib EnhancedBiasWeighterLib xAODEventInfo PathResolver TrigConfData )
+                     PRIVATE_LINK_LIBRARIES ${ROOT_LIBRARIES} GaudiKernel AthAnalysisBaseCompsLib TrigDataAccessMonitoringLib TrigDecisionToolLib EnhancedBiasWeighterLib xAODEventInfo PathResolver TrigConfData )
 
 # Small helper library used by trigCostHistToCSV
 atlas_add_library( TrigCostAnalysisLib
@@ -25,3 +25,4 @@ atlas_add_executable( trigCostHistToCSV
 
 atlas_install_joboptions( share/TrigCostAnalysis_JobOptions.py )
 atlas_install_scripts( share/RunTrigCostAnalysis.py POST_BUILD_CMD ${ATLAS_FLAKE8} )
+atlas_install_python_modules( python/*.py )

Trigger/TrigCost/TrigCostAnalysis/python/__init__.py

+# Copyright (C) 2002-2020 CERN for the benefit of the ATLAS collaboration
\ No newline at end of file

Trigger/TrigCost/TrigCostAnalysis/share/TrigCostAnalysis_JobOptions.py

@@ -58,6 +58,9 @@ enhancedBiasWeighter.RunNumber = 379158
 enhancedBiasWeighter.UseBunchCrossingTool = False
 
 from AthenaCommon import CfgMgr
+from TrigCostAnalysis.ROSToROB import ROSToROBMap
+rosToRobMap = ROSToROBMap()
+
 trigCostAnalysis = CfgMgr.TrigCostAnalysis()
 trigCostAnalysis.OutputLevel = DEBUG
 trigCostAnalysis.RootStreamName = "COSTSTREAM"
@@ -66,6 +69,7 @@ trigCostAnalysis.EnhancedBiasTool = enhancedBiasWeighter
 trigCostAnalysis.UseEBWeights = False
 trigCostAnalysis.MaxFullEventDumps = 100
 trigCostAnalysis.FullEventDumpProbability = 1 # X. Where probability is 1 in X
+trigCostAnalysis.ROSToROBMap = rosToRobMap.get_mapping()
 topSequence += trigCostAnalysis
 
 from AthenaCommon.AppMgr import ServiceMgr as svcMgr

Trigger/TrigCost/TrigCostAnalysis/src/CostData.cxx

@@ -46,6 +46,10 @@ StatusCode CostData::cache() {
   return StatusCode::SUCCESS;
 }
 
+void CostData::setRosToRobMap(const std::map<std::string, std::vector<uint32_t>>& rosToRobMap) {
+  m_rosToRob = &rosToRobMap;
+}
+
 void CostData::setLb(uint32_t lb) {
   m_lb = lb;
 }
@@ -80,6 +84,9 @@ const xAOD::TrigCompositeContainer& CostData::rosCollection() const {
   return *m_rosCollection;
 }
 
+const std::map<std::string, std::vector<uint32_t>>& CostData::rosToRobMap() const {
+  return *m_rosToRob;
+}
 
 float CostData::algTotalTimeMilliSec() const {
   return m_algTotalTime * 1e-3; // microseconds to milliseconds

Trigger/TrigCost/TrigCostAnalysis/src/CostData.h

@@ -42,7 +42,7 @@ class CostData {
     CostData(const CostData&) = delete;
 
     /**
-     * @brief Cache the cost collection, after formally requesting it from storegate.
+     * @brief Cache the cost and ros collections, after formally requesting it from storegate.
      */
     StatusCode set(const xAOD::TrigCompositeContainer* costCollection, const xAOD::TrigCompositeContainer* rosCollection, uint32_t onlineSlot);
 
@@ -56,6 +56,16 @@ class CostData {
      */
     const xAOD::TrigCompositeContainer& rosCollection() const;
 
+    /**
+     * @brief Getter of the ROS to ROB map.
+     */
+    const std::map<std::string, std::vector<uint32_t>>& rosToRobMap() const;
+
+    /**
+     * @brief Set ROS to ROB map
+     */
+    void setRosToRobMap(const std::map<std::string, std::vector<uint32_t>>& rosToRobMap);
+
     /**
      * @brief Setter of effective P1 walltime represented by the current event.
      */
@@ -134,6 +144,8 @@ class CostData {
     bool m_liveTimeIsPerEvent; //!< If the livetime represents a single event or all of the current LB
     const std::unordered_map<uint32_t, std::string>* m_typeMapPtr; //!< Cached non-owning pointer mapping algorithm instance names to types
     std::map<size_t, std::vector<size_t>> m_algToRos; //!< Mapping of indexes from m_costCollection to corresponding ROS requests made by algorithm
+    const std::map<std::string, std::vector<uint32_t>>* m_rosToRob; //!< Mapping of ROS corresponding to ROB requests
+
 };
 
 #endif // TRIGCOSTANALYSIS_COSTDATA_H

Trigger/TrigCost/TrigCostAnalysis/src/TrigCostAnalysis.cxx

@@ -152,6 +152,7 @@ StatusCode TrigCostAnalysis::execute() {
   const uint32_t onlineSlot = getOnlineSlot( costDataHandle.get() );
   CostData costData;
   ATH_CHECK( costData.set(costDataHandle.get(), rosDataHandle.get(), onlineSlot) );
+  costData.setRosToRobMap(m_rosToRob);
   costData.setLb( context.eventID().lumi_block() );
   costData.setTypeMap( m_algTypeMap );
   if (!m_enhancedBiasTool.name().empty()) {

Trigger/TrigCost/TrigCostAnalysis/src/TrigCostAnalysis.h

@@ -12,6 +12,8 @@
 
 #include "EnhancedBiasWeighter/EnhancedBiasWeighter.h"
 
+#include "Gaudi/Parsers/Factory.h"
+
 #include "MonitoredRange.h"
 
 #include <unordered_map>
@@ -111,6 +113,9 @@ class TrigCostAnalysis: public ::AthHistogramAlgorithm {
     Gaudi::Property<float> m_baseEventWeight { this, "BaseEventWeight", true,
       "Base events weight, other weights may be multiplied on top of this one." };
 
+    Gaudi::Property<std::map<std::string, std::vector<uint32_t>>> m_rosToRob {
+      this, "ROSToROBMap", {}, "ROS to ROB mapping" };
+
     SG::ReadHandleKey<xAOD::TrigCompositeContainer> m_costDataKey { this, "CostReadHandleKey", "HLT_TrigCostContainer",
       "Trigger cost payload container for algorithms" };
 

Trigger/TrigCost/TrigCostAnalysis/src/counters/CounterROS.cxx

@@ -3,36 +3,91 @@
 */
 
 #include "xAODTrigger/TrigCompositeContainer.h"
+#include "TrigDataAccessMonitoring/ROBDataMonitor.h"
 
 #include "CounterROS.h"
 
+#include <algorithm>
+#include <string>
+#include <vector>
+
 CounterROS::CounterROS(const std::string& name, const MonitorBase* parent) 
-  : CounterBase(name, parent)
-{}
+  : CounterBase(name, parent) {
+
+  regHistogram("Request_perEvent", "Number of requests/Event;Number of requests;Events", VariableType::kPerEvent, LogType::kLinear, -0.5, 10.5, 11);
+  regHistogram("NetworkRequest_perEvent", "Number of network requests/Event;Number of requests;Events", VariableType::kPerEvent, LogType::kLinear, -0.5, 10.5, 11);
+  regHistogram("CachedROBSize_perEvent", "Total ROB Size/Event;ROB size;Events", VariableType::kPerEvent, LogType::kLinear, 0, 1024, 50);
+  regHistogram("NetworkROBSize_perEvent", "Total ROB Size/Event;ROB size;Events", VariableType::kPerEvent, LogType::kLinear, 0, 1024, 50);
+  regHistogram("Time_perEvent", "ROB Elapsed Time/Event;Elapsed Time [ms];Events", VariableType::kPerEvent);
+  regHistogram("ROBStatus_perCall", "ROB status/Call;Status;Events", VariableType::kPerCall, LogType::kLinear, 0, robmonitor::NUM_ROBHIST_CODES+1, robmonitor::NUM_ROBHIST_CODES+1);
+}
+
+StatusCode CounterROS::newEvent(const CostData& data, size_t index, const float weight) {
+
+  // Monitor only ROB data for corresponding ROS
+  const xAOD::TrigComposite* tc = data.rosCollection()[index];
+  const std::vector<uint32_t> robIdsPerRequest = tc->getDetail<std::vector<uint32_t>>("robs_id");
+  const std::vector<uint32_t> robs_size = tc->getDetail<std::vector<uint32_t>>("robs_size");
+  const std::vector<unsigned> robs_history = tc->getDetail<std::vector<unsigned>>("robs_history");
+  const std::vector<uint8_t> robs_status = tc->getDetail<std::vector<uint8_t>>("robs_status");
+
+  if (m_robIdsPerROS.size() == 0) {
+    m_robIdsPerROS = data.rosToRobMap().at(getName());
+  }
+
+  // Find all ROB requests that are both in request and correspond to this ROS
+  bool networkRequestIncremented = false;
+  for (size_t i = 0; i < robIdsPerRequest.size(); ++i) {
+    if (std::find(m_robIdsPerROS.begin(), m_robIdsPerROS.end(), robIdsPerRequest[i]) != m_robIdsPerROS.end()) {
+
+      ATH_CHECK( fill("ROBStatus_perCall", getROBHistoryBin(robs_history[i]), weight) );
+      if (robs_status[i]) {
+        // The last bin of ROBStatus_perCall histogram store isStatusOk bool value
+        ATH_CHECK( fill("ROBStatus_perCall", robmonitor::NUM_ROBHIST_CODES, weight) );
+      }
+
+      // ROB request was fetched over the network
+      if (robs_history[i] == robmonitor::RETRIEVED) {
+        ATH_CHECK( fill("NetworkROBSize_perEvent", robs_size[i], weight) );
+        networkRequestIncremented = true;
+      }
+      else {
+        ATH_CHECK( fill("CachedROBSize_perEvent", robs_size[i], weight) );
+      }
+    }
+  }
+
+  ATH_CHECK( increment("Request_perEvent", weight) );
+
+  if (networkRequestIncremented) {
+    ATH_CHECK( increment("NetworkRequest_perEvent", weight) );
+  }
+
+  const float rosTime = timeToMilliSec(tc->getDetail<uint64_t>("start"), tc->getDetail<uint64_t>("stop"));
+  ATH_CHECK( fill("Time_perEvent", rosTime, weight) );
 
-StatusCode CounterROS::newEvent(const CostData& /*data*/, size_t /*incrementWalltime*/, const float /*weight*/) {
   return StatusCode::SUCCESS;
 }
 
 int CounterROS::getROBHistoryBin(const unsigned history){
   int history_bin;
   switch (history) {
-    case 1: // SCHEDULED
+    case robmonitor::SCHEDULED:
       history_bin = 1;
       break;
-    case 2: // RETRIEVED
-      history_bin = 1;
+    case robmonitor::RETRIEVED:
+      history_bin = 2;
       break;
-    case 4: // HLT_CACHED
+    case robmonitor::HLT_CACHED:
       history_bin = 3;
       break;
-    case 8: // DCM_CACHED
+    case robmonitor::DCM_CACHED:
       history_bin = 4;
       break;
-    case 16: // IGNORED
+    case robmonitor::IGNORED:
       history_bin = 5;
       break;
-    case 32: // DISABLED
+    case robmonitor::DISABLED:
       history_bin = 6;
       break;
     default: // UNCLASSIFIED 

Trigger/TrigCost/TrigCostAnalysis/src/counters/CounterROS.h

@@ -44,10 +44,10 @@ class CounterROS : public CounterBase {
     /**
      * @brief Concrete implementation. Monitors global properties in a single LB, or over all LB in a Range
      * @param[in] data Access to event data
-     * @param[in] incrementWalltime If 1, we should add the current events wall time to our internal histogram
+     * @param[in] index Index of data request to monitor
      * @param[in] weight Global event weight
      */
-    virtual StatusCode newEvent(const CostData& data, size_t incrementWalltime, const float weight = 1.) override;
+    virtual StatusCode newEvent(const CostData& data, size_t index, const float weight = 1.) override;
 
   private:
       /**
@@ -55,7 +55,8 @@ class CounterROS : public CounterBase {
      * @param[in] history ROBHistory value
      */
     int getROBHistoryBin(const unsigned history);
-    
+
+    std::vector<uint32_t> m_robIdsPerROS; //!< Cached mapping of ROB ids corresponding to ROS
 };
 
 #endif // TRIGCOSTANALYSIS_COUNTERROS_H
\ No newline at end of file

Trigger/TrigCost/TrigCostAnalysis/src/monitors/MonitorROS.cxx

@@ -5,12 +5,38 @@
 #include "MonitorROS.h"
 #include "../counters/CounterROS.h"
 
+#include <algorithm>
+
 MonitorROS::MonitorROS(const std::string& name, const MonitoredRange* parent)
   : MonitorBase(name, parent) {
 }
 
 
-StatusCode MonitorROS::newEvent(const CostData& /*data*/, const float /*weight*/) {
+StatusCode MonitorROS::newEvent(const CostData& data, const float weight) {
+
+  // Prepare ROB id per corresponding ROS name map
+  if (m_robToRos.empty()) {
+    const std::map<std::string, std::vector<uint32_t>> rosToRobMap = data.rosToRobMap();
+    for (auto rosRequest : rosToRobMap) {
+      for (uint32_t robId : rosRequest.second) {
+        m_robToRos[robId] = rosRequest.first;
+      }
+    }
+  }
+
+  for (const xAOD::TrigComposite* tc : data.rosCollection()) {
+    auto robIds = tc->getDetail<std::vector<uint32_t>>("robs_id");
+
+    // Create set of unique ROS for this request
+    std::set<std::string> rosPerRequest;
+    for (uint32_t robId : robIds) {
+      rosPerRequest.insert(m_robToRos[robId]);
+    }
+
+    for (const std::string& rosName : rosPerRequest) {
+      ATH_CHECK( getCounter(rosName)->newEvent(data, tc->index(), weight) );
+    }
+  }
 
   return StatusCode::SUCCESS;
 }

Trigger/TrigCost/TrigCostAnalysis/src/monitors/MonitorROS.h

@@ -53,6 +53,9 @@ class MonitorROS : public MonitorBase {
      * @return Owning unique ptr object typed on the CounterBase base class which points to concrete Counter of specialised type.
      */
     virtual std::unique_ptr<CounterBase> newCounter(const std::string& name) override; 
+
+  private:
+    std::map<uint32_t, std::string> m_robToRos; //!< Cache correspondis ROS per ROB id
 };
 
 #endif // TRIGCOSTANALYSIS_MONITORROS_H
\ No newline at end of file

Trigger/TriggerCommon/TrigEDMConfig/python/TriggerEDMRun3.py

@@ -97,7 +97,7 @@ TriggerHLTListRun3 = [
     ('xAOD::TrigCompositeContainer#HLT_TrigCostContainer',   'CostMonDS ESD', 'Steer'),
     ('xAOD::TrigCompositeAuxContainer#HLT_TrigCostContainerAux.alg.store.view.thread.thash.slot.roi.start.stop', 'CostMonDS ESD', 'Steer'),
     ('xAOD::TrigCompositeContainer#HLT_TrigCostROSContainer',   'CostMonDS ESD', 'Steer'),
-    ('xAOD::TrigCompositeAuxContainer#HLT_TrigCostROSContainerAux.alg_idx.lvl1ID.robs_is.robs_size.robs_history.robs_status.start.stop', 'CostMonDS ESD', 'Steer'),
+    ('xAOD::TrigCompositeAuxContainer#HLT_TrigCostROSContainerAux.alg_idx.lvl1ID.robs_id.robs_size.robs_history.robs_status.start.stop', 'CostMonDS ESD', 'Steer'),
 
     # Run-2 L1 (temporary)
     ('xAOD::MuonRoIContainer#LVL1MuonRoIs' ,                 'ESD AODFULL AODSLIM AODVERYSLIM AODBLSSLIM', 'L1'),