From 1ce9fb13cb0e616b1ebdf305aa30bb0dc5260601 Mon Sep 17 00:00:00 2001
From: Frank Winklmeier <Frank.Winklmeier@cern.ch>
Date: Mon, 11 Sep 2017 10:29:03 +0200
Subject: [PATCH] Use new-style properties to declare empty monitoring
 ToolHandle

Thanks to atlas/Gaudi!149 and atlas/athena!4537, we can now define empty
ToolHandles in header files as well. Other changes:
- Update doxygen
- Allow 'list' in labels argument to defineHistogram
- Add 2D histogram example to AthExMonitored
---
 .../AthExMonitored/share/MonitoredOptions.py  |  2 +
 .../AthExMonitored/src/MonitoredAlg.cxx       |  3 --
 .../AthExMonitored/src/MonitoredAlg.h         |  2 +-
 .../AthenaMonitoring/GenericMonitoringTool.h  | 33 ++++++++-----
 Control/AthenaMonitoring/doc/Monitored_page.h | 48 +++++++++++++------
 .../python/GenericMonitoringTool.py           | 19 ++++++--
 Control/AthenaMonitoring/src/HistogramDef.cxx |  6 +--
 .../src/TrigL2ElectronHypoTool.cxx            |  3 +-
 .../src/TrigL2ElectronHypoTool.h              |  2 +-
 9 files changed, 78 insertions(+), 40 deletions(-)

diff --git a/Control/AthenaExamples/AthExMonitored/share/MonitoredOptions.py b/Control/AthenaExamples/AthExMonitored/share/MonitoredOptions.py
index 357d140a322..e55b216521c 100644
--- a/Control/AthenaExamples/AthExMonitored/share/MonitoredOptions.py
+++ b/Control/AthenaExamples/AthExMonitored/share/MonitoredOptions.py
@@ -18,6 +18,8 @@ monTool.Histograms = [defineHistogram( 'nTracks', path='EXPERT', type='TH1F', ti
                                        xbins=30, xmin=-3, xmax=3 ),
                       defineHistogram( 'AbsPhi', path='EXPERT', type='TH1F', title='|#phi|;;Entries',
                                        xbins=10, xmin=0, xmax=3.15 ),
+                      defineHistogram( 'eta,AbsPhi', path='EXPERT', type='TH2F', title='#eta vs #phi',
+                                       xbins=15, xmin=-3, xmax=3, ybins=15, ymin=0, ymax=3.15 ),
                       defineHistogram( 'TIME_execute', path='EXPERT', type='TH1F', title='Time for execute',
                                        xbins=100, xmin=0, xmax=100 ) ]
 
diff --git a/Control/AthenaExamples/AthExMonitored/src/MonitoredAlg.cxx b/Control/AthenaExamples/AthExMonitored/src/MonitoredAlg.cxx
index 7ae4db82aef..5cfebe8faed 100644
--- a/Control/AthenaExamples/AthExMonitored/src/MonitoredAlg.cxx
+++ b/Control/AthenaExamples/AthExMonitored/src/MonitoredAlg.cxx
@@ -10,9 +10,6 @@
 MonitoredAlg::MonitoredAlg(const std::string& name, ISvcLocator* pSvcLocator) :
   AthAlgorithm(name, pSvcLocator)
 {
-  // By default set an empty monitoring tool
-  // TODO: Replace this with a .h property once Gaudi!385 has been merged
-  declareProperty("MonTool", m_monTool=VoidMonitoringTool(this), "Monitoring tool");
 }
 
 StatusCode MonitoredAlg::initialize()
diff --git a/Control/AthenaExamples/AthExMonitored/src/MonitoredAlg.h b/Control/AthenaExamples/AthExMonitored/src/MonitoredAlg.h
index 227cc83d287..0760c17d7e4 100644
--- a/Control/AthenaExamples/AthExMonitored/src/MonitoredAlg.h
+++ b/Control/AthenaExamples/AthExMonitored/src/MonitoredAlg.h
@@ -19,7 +19,7 @@ public:
   StatusCode execute();
 
 private:
-  ToolHandle<GenericMonitoringTool> m_monTool;
+  ToolHandle<GenericMonitoringTool> m_monTool{this,"MonTool","","Monitoring tool"};
 };
 
 
diff --git a/Control/AthenaMonitoring/AthenaMonitoring/GenericMonitoringTool.h b/Control/AthenaMonitoring/AthenaMonitoring/GenericMonitoringTool.h
index df8f64e1406..5acff25e1c8 100644
--- a/Control/AthenaMonitoring/AthenaMonitoring/GenericMonitoringTool.h
+++ b/Control/AthenaMonitoring/AthenaMonitoring/GenericMonitoringTool.h
@@ -28,28 +28,37 @@
  *
  * The tool can be used standalone or attached to an algorithm, tool or service.
  * The variables to be monitored need to be exposed via the Monitored framework.
- * The histograms to be created are configured via the `Histograms` property of the form:
+ * The histograms to be created are configured via the `Histograms` list property.
+ * Each list entry has the form:
  *
- *   PATH, HTYPE, NAME, TITLE, BINNING ...., OPTIONS
+ *   "PATH, HTYPE, NAME, TITLE, BINNING, [LABELS], OPTIONS"
  *
  * Example configuration strings:
- * - `"/SHIFT, TH1D, name, title, xbins, xmin, xmax, opt"`
- * - `"/EXPERT, TH2D, "name1,name2", title, xbins, xmin, xmax, ybins, ymin, ymax, opt"`
+ * - `"SHIFT, TH1D, name, title, xbins, xmin, xmax, opt"`
+ * - `"EXPERT, TH2D, "name1,name2", title, xbins, xmin, xmax, ybins, ymin, ymax, opt"`
  *
  * As an option an alias can be provided for the naming of the actual histogram:
  *   `"name;alias"` or `"name1,name2;alias"`
- * For the configuration from python the helper DefineHistogram.defineHistogram should be used.
+ * For the configuration from python the helper GenericMonitoringTool.defineHistogram should be used.
  *
- * The following histogram types are supported: TH1[F,D,I], TH2[F,D,I], TProfile, TProfile2D
+ * The following histogram types are supported:
+ * - TH1[F,D,I]
+ * - TH2[F,D,I]
+ * - TProfile[2D]
  *
- * The following top-level paths are supported: EXPERT, SHIFT, DEBUG, RUNSTAT, EXPRESS
+ * The following top-level paths are supported:
+ * - EXPERT, SHIFT, DEBUG, RUNSTAT, EXPRESS
  *
  * The following options are suppored:
- * - kCanRebin enables ROOT's internal functionality of autobinning the histogram
- * - kCumulative does fill of all bins left to the bin into which the value falls
- * - kLBN makes the histogram lumiblock aware
- * - kVec adds the content of the monitored variable to the histogram bins
- * - kVecUO same as kVec but treat 0th(last) element as underflow(overflow)
+ * - `kCanRebin` enables ROOT's internal functionality of autobinning the histogram
+ * - `kCumulative` does fill of all bins left to the bin into which the value falls
+ * - `kLBN` makes the histogram lumiblock aware
+ * - `kVec` adds the content of the monitored variable to the histogram bins
+ * - `kVecUO` same as kVec but treat 0th(last) element as underflow(overflow)
+ *
+ * Optionally, a colon-separated list of bin labels ("bin1:bin2:bin3:") can be provided (at least one
+ * colon is required). In case of a 2D histogram the labels are assigned consecutively to the x-axis 
+ * and then y-axis bins.
  *
  * @author Tomasz Bold
  * @author Piotr Sarna
diff --git a/Control/AthenaMonitoring/doc/Monitored_page.h b/Control/AthenaMonitoring/doc/Monitored_page.h
index b739c270a83..fd1bef8af34 100644
--- a/Control/AthenaMonitoring/doc/Monitored_page.h
+++ b/Control/AthenaMonitoring/doc/Monitored_page.h
@@ -9,9 +9,10 @@
    single-threaded athena. To make use of this infrastructure the following
    steps are needed:
    
-   1) Add a GenericMonitoringTool instace to your component
+   1) Add a GenericMonitoringTool instance to your component
    \code
-      declareProperty("MonTool", m_monTool=VoidMonitoringTool(this), "Monitoring tool");
+    private:
+      ToolHandle<GenericMonitoringTool> m_monTool{this,"MonTool","","Monitoring tool"};
    \endcode
    Note that by default we are using an empty ToolHandle, i.e. there is no
    monitoring tool attached by default. This should instead be done in the
@@ -21,33 +22,50 @@
       myalg.MonTool = GenericMonitoringTool('MonTool')
    \endcode
 
-   2) Retrieve the monitoring tool during initialize() if the ToolHandle is not
-   empty:
+   2) Retrieve the monitoring tool during `initialize()` **if the ToolHandle is not empty**:
    \code
       if (!m_monTool.empty()) CHECK(m_monTool.retrieve());
    \endcode
+   The additional check is needed because retrieval of an emtpy ToolHandle results in a failure.
 
    3) Declare the monitored quantities to the monitoring framework.
-   Several classes are available to export different types to the monitoring
-   framwork: See the detailed documentation of Monitored::MonitoredScalar,
-   Monitored::MonitoredCollection or Monitored::MonitoredTimer. For example, a monitored scalar object
-   is declared via Monitored::MonitoredScalar::declare(std::string name, const T&
-   defaultVaule):
+   Several classes are available to export different types to the monitoring framwork: 
+     - Monitored::MonitoredScalar
+     - Monitored::MonitoredCollection
+     - Monitored::MonitoredTimer
 
-   @copydetails Monitored::MonitoredScalar::declare(std::string name, const T& defaultVaule)
+  The declaration in all cases is done via the `declare` method in the relevant namespace.
+  For example to declare a simple scalar, use:
+   \code
+   Monitored::MonitoredScalar::declare(std::string name, const T& defaultVaule):
+   \endcode
 
-   4) Group monitored variables within a Monitored::MonitoredScope that will take care of
-   filling histogram once it goes out of scope:
+   @copydetails Monitored::MonitoredScalar::declare(std::string name, const T& defaultVaule)
+   
+   All above functions are within the Monitored namespace. Consider adding
+   \code using namespace Monitored;\endcode
+   to your function (but avoid doing this at global scope).
 
+   4)
    @copydoc Monitored::MonitoredScope
 
    5) Configure the list of histograms in python
    \code
-      from AthenaMonitoring.GenericMonitoringTool import defineHistogram
-      monTool.Histograms = [defineHistogram('nTracks', path='EXPERT', type='TH1F', title='Counts',
-                                             xbins=10, xmin=0, xmax=10)]
+      from AthenaMonitoring.GenericMonitoringTool import GenericMonitoringTool, defineHistogram
+      monTool = GenericMonitoringTool('MonTool')
+      monTool.Histograms = [defineHistogram('eta', path='EXPERT', type='TH1F', title='Eta;;Entries',
+                                             xbins=40, xmin=-2, xmax=2),
+                            defineHistogram('phi', path='EXPERT', type='TH1F', title='Phi;;Entries',
+                                             xbins=60, xmin=-3.2, xmax=3.2),
+                            defineHistogram('eta,phi', path='EXPERT', type='TH2F', title='Eta vs Phi',
+                                             xbins=20, xmin=-2, xmax=2, ybins=30, ymin=-3.2, ymax=3.2)]
+
+     topSequence.myAlg.MonTool = monTool
    \endcode
 
+   \remark Without this python configuration, i.e. the last line, no monitoring tool is instantiated
+   and no monitoring histograms created thus reducing the overhead (both time and memory) to a minimum.
+                                             
    Additional documentation:
    - The MonitoredAlg standalone example and its MonitoredOptions.py job
    options
diff --git a/Control/AthenaMonitoring/python/GenericMonitoringTool.py b/Control/AthenaMonitoring/python/GenericMonitoringTool.py
index 08db881b89d..2b2a2658f71 100644
--- a/Control/AthenaMonitoring/python/GenericMonitoringTool.py
+++ b/Control/AthenaMonitoring/python/GenericMonitoringTool.py
@@ -11,14 +11,20 @@ class GenericMonitoringTool(_GenericMonitoringTool):
         super(GenericMonitoringTool, self).__init__(name, **kwargs)
 
 
+## Generate histogram definition string for the `GenericMonitoringTool.Histograms` property
+#
+#  For full details see the GenericMonitoringTool documentation.
+#  @param varname  one (1D) or two (2D) variable names separated by comma
+#  @param type     histogram type
+#  @param path     top-level histrogram directory
+#  @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='EXPERT',
                     title=None,
                     xbins=100, xmin=0, xmax=1,
                     ybins=None, ymin=None, ymax=None, zmin=None, zmax=None, opt='', labels=None):
-    """ Generates the histogram definition string which is digestable by GenericMonitoringTool.
 
-    See the GenericMonitoringTool documentation for histogram types, paths and other options. 
-    """
     if title is None: title=varname
     coded = "%s, %s, %s, %s, %d, %f, %f" % (path, type, varname, title, xbins, xmin, xmax)
     if ybins is not None:
@@ -27,7 +33,12 @@ def defineHistogram(varname, type='TH1F', path='EXPERT',
             coded += ", %f, %f" % (zmin, zmax)
     if ybins is None and ymin is not None:
         coded += ", %f, %f" % (ymin, ymax)
-    if labels is not None:
+
+    if isinstance(labels, list) and len(labels)>0:
+        coded += ',' + ':'.join(labels) + ':'
+
+    # For backwards compatibility
+    elif labels is not None:
         labels = labels.strip()   # remove spurious white-spaces
         if len(labels)>0:
             if labels[-1]!=':': labels += ':'  # C++ parser expects at least one ":"
diff --git a/Control/AthenaMonitoring/src/HistogramDef.cxx b/Control/AthenaMonitoring/src/HistogramDef.cxx
index 73d1add958b..5c7b977d05b 100644
--- a/Control/AthenaMonitoring/src/HistogramDef.cxx
+++ b/Control/AthenaMonitoring/src/HistogramDef.cxx
@@ -229,8 +229,8 @@ const HistogramDef HistogramDef::parse(const std::string& jobOpts) {
     }
   }
 
-  if (itr->find(":") != std::string::npos ) { // it means that last paramater has format str1:str2:str3:str4 which means this are bins labels
-    // breate it 
+  if (itr->find(":") != std::string::npos ) { // it means that last paramater has format str1:str2:str3:str4 which means these are bins labels
+    // split it 
     boost::char_separator<char> colon(":");
     tokenizer_t labels(*itr, colon);
     for ( tokenizer_t::iterator l = labels.begin(); l != labels.end(); ++l ) {
@@ -246,4 +246,4 @@ const HistogramDef HistogramDef::parse(const std::string& jobOpts) {
     
   histPar.ok = true;
   return histPar;
-}
\ No newline at end of file
+}
diff --git a/Trigger/TrigHypothesis/TrigEgammaHypo/src/TrigL2ElectronHypoTool.cxx b/Trigger/TrigHypothesis/TrigEgammaHypo/src/TrigL2ElectronHypoTool.cxx
index 45865f38982..9478ccb12c7 100644
--- a/Trigger/TrigHypothesis/TrigEgammaHypo/src/TrigL2ElectronHypoTool.cxx
+++ b/Trigger/TrigHypothesis/TrigEgammaHypo/src/TrigL2ElectronHypoTool.cxx
@@ -18,8 +18,9 @@ TrigL2ElectronHypoTool::TrigL2ElectronHypoTool( const std::string& type,
 {}
 
 StatusCode TrigL2ElectronHypoTool::initialize()  {
-  ATH_MSG_DEBUG( "Initialization:" );
   
+  if (!m_monTool.empty()) CHECK(m_monTool.retrieve());
+
   ATH_MSG_DEBUG( "Initialization completed successfully:" );
   ATH_MSG_DEBUG( "AcceptAll            = " 
 		<< ( m_acceptAll==true ? "True" : "False" ) ); 
diff --git a/Trigger/TrigHypothesis/TrigEgammaHypo/src/TrigL2ElectronHypoTool.h b/Trigger/TrigHypothesis/TrigEgammaHypo/src/TrigL2ElectronHypoTool.h
index 0418e2cf8dd..fa6828d0e3e 100644
--- a/Trigger/TrigHypothesis/TrigEgammaHypo/src/TrigL2ElectronHypoTool.h
+++ b/Trigger/TrigHypothesis/TrigEgammaHypo/src/TrigL2ElectronHypoTool.h
@@ -90,7 +90,7 @@ class TrigL2ElectronHypoTool
 
   size_t m_multiplicity = 1;
 
-  ToolHandle<GenericMonitoringTool> m_monTool{ this, "MonTool", "GenericMonitoringTool/MOnTool", "Monitoring tool"};
+  ToolHandle<GenericMonitoringTool> m_monTool{ this, "MonTool", "", "Monitoring tool" };
 }; 
 
 inline const InterfaceID& TrigL2ElectronHypoTool::interfaceID() 
-- 
GitLab