From 37aa860bb42dac7b0f6afebbd02b5f187ec77cb9 Mon Sep 17 00:00:00 2001
From: Charles Burton <cdb97@cornell.edu>
Date: Wed, 23 Jan 2019 09:55:06 -0600
Subject: [PATCH] resolve issues with offline monitoring MR

1. implement a manual fill wrapper for offline monitoring
2. revert change in GenericMonitoringTool default path to 'EXPERT'. This will
   be fixed in another MR
3. Update example AthMonitorAlgorithm daughter class for changes to base class
---
 .../AthenaMonitoring/AthMonitorAlgorithm.h    | 42 ++++++++++++++++---
 .../ExampleMonitorAlgorithm.h                 |  5 +--
 .../python/GenericMonitoringTool.py           |  2 +-
 .../src/AthMonitorAlgorithm.cxx               | 15 ++++---
 .../src/ExampleMonitorAlgorithm.cxx           | 15 ++++---
 5 files changed, 56 insertions(+), 23 deletions(-)

diff --git a/Control/AthenaMonitoring/AthenaMonitoring/AthMonitorAlgorithm.h b/Control/AthenaMonitoring/AthenaMonitoring/AthMonitorAlgorithm.h
index cdb1d6b12cd..7154ccad707 100644
--- a/Control/AthenaMonitoring/AthenaMonitoring/AthMonitorAlgorithm.h
+++ b/Control/AthenaMonitoring/AthenaMonitoring/AthMonitorAlgorithm.h
@@ -23,6 +23,7 @@
 #include "AthenaMonitoring/IDQFilterTool.h"
 #include "AthenaMonitoring/IMonitorToolBase.h"
 #include "AthenaMonitoring/ITriggerTranslatorTool.h"
+#include "AthenaMonitoring/Monitored.h"
 
 #include "LumiBlockComps/ILuminosityTool.h"
 #include "TrigDecisionInterface/ITrigDecisionTool.h"
@@ -34,7 +35,6 @@
 class AthMonitorAlgorithm : public AthReentrantAlgorithm {
 public:
 
-
     /** 
      * Constructor
      */
@@ -76,6 +76,38 @@ public:
     virtual StatusCode fillHistograms(const EventContext& ctx) const = 0;
 
 
+    /**
+     * Adds variables from an event to a group by name.
+     * 
+     * At the end of the fillHistograms routine, one should save the monitored variables 
+     * to the group. This function wraps the process of getting the desired group by a 
+     * call to AthMonitorAlgorithm::getGroup() and a call to Monitored::Group::fill(), 
+     * which also disables the auto-fill feature to avoid double-filling. Note, users 
+     * should avoid using this specific function name in daughter classes.
+     * 
+     * @param groupName The string name of the GenericMonitoringTool
+     * @param variables Variables desired to be saved.
+     * @return StatusCode
+     */
+    template <typename... T>
+    void fill( const std::string& groupName, T&&... variables ) const {
+        Monitored::Group(getGroup(groupName),std::forward<T>(variables)...).fill();
+    }
+
+
+    /**
+     * Adds variables from an event to a group by object reference.
+     * 
+     * @param groupHandle A reference of the GenericMonitoringTool to which add variables.
+     * @param variables Variables desired to be saved
+     * @return StatusCode
+     */
+    template <typename... T>
+    void fill( const ToolHandle<GenericMonitoringTool>& groupHandle, T&&... variables ) const {
+        Monitored::Group(groupHandle,std::forward<T>(variables)...).fill();
+    }
+
+
     /**
      * Specifies the processing environment.
      * 
@@ -148,14 +180,13 @@ public:
      * Get a specific monitoring tool from the tool handle array.
      * 
      * Finds a specific GenericMonitoringTool instance from the list of monitoring
-     * tools (a ToolHandleArray). Throws a FATAL warning if the object found is a
-     * null pointer.
+     * tools (a ToolHandleArray). Throws a FATAL warning if the object found is 
+     * empty.
      * 
      * @param name string name of the desired tool
      * @return reference to the desired monitoring tool
      */
-    GenericMonitoringTool& getGroup( const std::string& name ) const;
-
+    ToolHandle<GenericMonitoringTool> getGroup( const std::string& name ) const;
 
     /** 
      * Get the trigger decision tool member.
@@ -262,7 +293,6 @@ public:
 
 protected:
     // Using the new way to declare JO properties: Gaudi::Property<int> m_myProperty {this,"MyProperty",0};
-
     ToolHandleArray<GenericMonitoringTool> m_tools {this,"GMTools",{}}; ///< Array of Generic Monitoring Tools
     ToolHandle<Trig::ITrigDecisionTool> m_trigDecTool {this,"TrigDecisionTool",""}; ///< Tool to tell whether a specific trigger is passed
     ToolHandle<ITriggerTranslatorTool> m_trigTranslator {this,"TriggerTranslatorTool",""}; ///< Tool to unpack trigger categories into a trigger list
diff --git a/Control/AthenaMonitoring/AthenaMonitoring/ExampleMonitorAlgorithm.h b/Control/AthenaMonitoring/AthenaMonitoring/ExampleMonitorAlgorithm.h
index 0d959860d56..c08f685c982 100644
--- a/Control/AthenaMonitoring/AthenaMonitoring/ExampleMonitorAlgorithm.h
+++ b/Control/AthenaMonitoring/AthenaMonitoring/ExampleMonitorAlgorithm.h
@@ -6,10 +6,7 @@
 #define EXAMPLEMONITORALGORITHM_H
 
 #include "AthenaMonitoring/AthMonitorAlgorithm.h"
-
-#include "AthenaMonitoring/MonitoredScope.h"
-#include "AthenaMonitoring/MonitoredScalar.h"
-#include "AthenaMonitoring/MonitoredCollection.h"
+#include "AthenaMonitoring/Monitored.h"
 
 #include "TRandom3.h"
 
diff --git a/Control/AthenaMonitoring/python/GenericMonitoringTool.py b/Control/AthenaMonitoring/python/GenericMonitoringTool.py
index 6ee23ef7663..d291ed59def 100644
--- a/Control/AthenaMonitoring/python/GenericMonitoringTool.py
+++ b/Control/AthenaMonitoring/python/GenericMonitoringTool.py
@@ -25,7 +25,7 @@ class GenericMonitoringTool(_GenericMonitoringTool):
 #  @param title    Histogram title and optional axis title (same syntax as in TH constructor)
 #  @param opt      Histrogram options (see GenericMonitoringTool)
 #  @param labels   List of bin labels (for a 2D histogram, sequential list of x- and y-axis labels)
-def defineHistogram(varname, type='TH1F', path='DEFAULT',
+def defineHistogram(varname, type='TH1F', path='EXPERT',
                     title=None,
                     xbins=100, xmin=0, xmax=1,
                     ybins=None, ymin=None, ymax=None, zmin=None, zmax=None, opt='', labels=None):
diff --git a/Control/AthenaMonitoring/src/AthMonitorAlgorithm.cxx b/Control/AthenaMonitoring/src/AthMonitorAlgorithm.cxx
index 044cf89f220..bcfda26e746 100644
--- a/Control/AthenaMonitoring/src/AthMonitorAlgorithm.cxx
+++ b/Control/AthenaMonitoring/src/AthMonitorAlgorithm.cxx
@@ -93,8 +93,9 @@ StatusCode AthMonitorAlgorithm::execute( const EventContext& ctx ) const {
     return fillHistograms(ctx);
 }
 
+
 SG::ReadHandle<xAOD::EventInfo> AthMonitorAlgorithm::GetEventInfo( const EventContext& ctx ) const {
-  return SG::ReadHandle<xAOD::EventInfo>(m_EventInfoKey, ctx);
+    return SG::ReadHandle<xAOD::EventInfo>(m_EventInfoKey, ctx);
 }
 
 
@@ -160,16 +161,18 @@ AthMonitorAlgorithm::DataType_t AthMonitorAlgorithm::dataTypeStringToEnum( const
 }
 
 
-GenericMonitoringTool& AthMonitorAlgorithm::getGroup( const std::string& name ) const {
-    // get the pointer to the tool, check that it exists, and return
-    GenericMonitoringTool* tool = &(*(*m_tools[name]));
-    if (tool == nullptr) {
+ToolHandle<GenericMonitoringTool> AthMonitorAlgorithm::getGroup( const std::string& name ) const {
+    // get the pointer to the tool, and check that it exists
+    const ToolHandle<GenericMonitoringTool> toolHandle = *(m_tools[name]);
+    if ( toolHandle.empty() ) {
         ATH_MSG_FATAL("The tool "<<name<<" could not be found in the monitoring algorithm's tool array."<<endmsg);
     }
-    return *tool;
+    // return the tool handle
+    return toolHandle;
 }
 
 
+
 const ToolHandle<Trig::ITrigDecisionTool>& AthMonitorAlgorithm::getTrigDecisionTool() {
     return m_trigDecTool;
 }
diff --git a/Control/AthenaMonitoring/src/ExampleMonitorAlgorithm.cxx b/Control/AthenaMonitoring/src/ExampleMonitorAlgorithm.cxx
index bc0167f758a..5fd6a5c9e91 100644
--- a/Control/AthenaMonitoring/src/ExampleMonitorAlgorithm.cxx
+++ b/Control/AthenaMonitoring/src/ExampleMonitorAlgorithm.cxx
@@ -24,20 +24,23 @@ StatusCode ExampleMonitorAlgorithm::fillHistograms( const EventContext& ctx ) co
     // Declare the quantities which should be monitored
     auto lumiPerBCID = MonitoredScalar::declare<float>("lumiPerBCID",0.0);
     auto lb = MonitoredScalar::declare<int>("lb",0);
-    auto random = MonitoredScalar::declare<float>("random",0.0);    
-    
+    auto run = MonitoredScalar::declare<int>("run",0);
+    auto random = MonitoredScalar::declare<float>("random",0.0);
     // Set the values of the monitored variables for the event
     lumiPerBCID = lbAverageInteractionsPerCrossing();
     lb = GetEventInfo(ctx)->lumiBlock();
+    run = GetEventInfo(ctx)->runNumber();
     if (m_doRandom) {
         TRandom r;
         random = r.Rndm();
     }
 
-    // Use the getGroup method to get your instance of the GenericMonitoringTool by name
-    auto& tool = getGroup("ExampleMonitor");
-    // Save. First argument is the tool, all others are the variables to be saved.
-    Monitored::save(&tool,lumiPerBCID,lb,random);
+    // Fill. First argument is the tool name, all others are the variables to be saved.
+    fill("ExampleMonitor",lumiPerBCID,lb,random);
+
+    // Alternative fill method. Get the group yourself, and pass it to the fill function.
+    auto tool = getGroup("ExampleMonitor");
+    fill(tool,run);
     
     return StatusCode::SUCCESS;
 }
-- 
GitLab