From 490647bc4e9348c2f8f9f3bcd70dc58bab7fb9f1 Mon Sep 17 00:00:00 2001
From: Sebastien Ponce <sebastien.ponce@cern.ch>
Date: Sat, 25 Mar 2023 14:23:03 +0100
Subject: [PATCH] Improved output of test-genfsr test and fix GenFSR merging

---
 .../tests/qmtest/fsrs.qms/test-genfsr.qmt     | 28 ++++++++----
 Phys/DaVinci/python/DaVinci/algorithms.py     | 45 ++++++++++++-------
 Phys/DaVinci/python/DaVinci/config.py         | 13 +-----
 Phys/DaVinci/tests/config/test_algorithms.py  |  4 +-
 4 files changed, 52 insertions(+), 38 deletions(-)

diff --git a/DaVinciTests/tests/qmtest/fsrs.qms/test-genfsr.qmt b/DaVinciTests/tests/qmtest/fsrs.qms/test-genfsr.qmt
index 8c6f79b98..b73b5037e 100644
--- a/DaVinciTests/tests/qmtest/fsrs.qms/test-genfsr.qmt
+++ b/DaVinciTests/tests/qmtest/fsrs.qms/test-genfsr.qmt
@@ -1,6 +1,6 @@
 <?xml version="1.0" ?><!DOCTYPE extension  PUBLIC '-//QM/2.3/Extension//EN'  'http://www.codesourcery.com/qm/dtds/2.3/-//qm/2.3/extension//en.dtd'>
 <!--
-    (c) Copyright 2000-2021 CERN for the benefit of the LHCb Collaboration
+    (c) Copyright 2000-2023 CERN for the benefit of the LHCb Collaboration
 
     This software is distributed under the terms of the GNU General Public
     Licence version 3 (GPL Version 3), copied verbatim in the file "COPYING".
@@ -26,26 +26,38 @@ file = ET.parse('GeneratorLogFSR.xml')
 root = file.getroot()
 
 expected_file = ET.parse(os.path.expandvars('../refs/GeneratorLogFSR_expected.xml'))
-
 expected_root = expected_file.getroot()
 
-flag = True
+missingCounters = []
+wrongCounters = []
 
 for expected_counter in expected_root.findall('counter'):
         expected_name = expected_counter.get('name')
         expected_value = int(expected_counter.find('value').text)
 
+        value = None
         for counter in root.findall('counter'):
                 name = counter.get('name')
                 if name == expected_name:
                         value = int(counter.find('value').text)
 
-        if value != expected_value:
-                flag = False
-                break
+        if value is None:
+                missingCounters.append(expected_name)
+        elif value != expected_value:
+                wrongCounters.append((expected_name, expected_value, value))
 
-if flag == False:
-        causes.append('Counter value different from what expected')
+if missingCounters or wrongCounters:
+        causes.append('Counters different from what expected')
+        errors = ''
+        if missingCounters:
+                errors += 'Missing counters:\n    ' + '\n    '.join(missingCounters)
+        if wrongCounters:
+                if missingCounters:
+                        errors += "\n"
+                errors += 'Wrong counters:\n    ' + '\n    '.join(['%s %d -> %d' % (name, expValue, value) for name, expValue, value in wrongCounters])
+        result['Expected XML'] = ET.tostring(expected_root).decode()
+        result['Output XML'] = ET.tostring(root).decode()
+        result['Counters mismatch'] = errors
 
 if os.path.exists('GeneratorLogFSR.xml'):
     os.remove('GeneratorLogFSR.xml')
diff --git a/Phys/DaVinci/python/DaVinci/algorithms.py b/Phys/DaVinci/python/DaVinci/algorithms.py
index 6380b8954..9d9e03280 100644
--- a/Phys/DaVinci/python/DaVinci/algorithms.py
+++ b/Phys/DaVinci/python/DaVinci/algorithms.py
@@ -119,9 +119,9 @@ def apply_filters(options, algs_dict):
     return alg_filterd_dict
 
 
-def define_fsr_writer(options):
+def make_fsr_algs(options):
     """
-    Define Generator FSR writer.
+    Make FSR related algorithms.
 
     Args:
         options (DaVinci.Options): lbexec provided options object
@@ -129,21 +129,32 @@ def define_fsr_writer(options):
     Returns:
         List of FSR algorithm instances to be configured.
     """
-    from PyConf.Algorithms import GenFSRMerge, RecordStream
-
-    algs = []
-    if options.merge_genfsr and options.simulation:
-        mergeGenfsr = GenFSRMerge(name="GenFSRMerge")
-        algs.append(mergeGenfsr)
-
-    outputLevel = options.output_level
-    recStream = RecordStream(
-        name="FSROutputStreamDstWriter",
-        OutputLevel=outputLevel,
-        Output="SVC='Gaudi::RootCnvSvc'")
-    algs.append(recStream)
-
-    return algs
+    from PyConf.Algorithms import EventAccounting, GenFSRMerge, RecordStream
+
+    fsrAlgs = {}
+
+    if options.simulation:
+        algs = []
+        if options.merge_genfsr:
+            algs.append(GenFSRMerge(name="GenFSRMerge"))
+        if options.write_fsr and options.output_file:
+            if not options.merge_genfsr:
+                # I have no idea if this is safe to do so raise an exception
+                raise NotImplementedError(
+                    "FSR output requested but no merging of FSRs performed.")
+            algs.append(
+                RecordStream(
+                    name="FSROutputStreamDstWriter",
+                    OutputLevel=options.output_level,
+                    Output="SVC='Gaudi::RootCnvSvc'"))
+        if algs:
+            fsrAlgs.update({"GenFSR": algs})
+
+    if options.lumi:
+        fsrAlgs.update({"Lumi": [EventAccounting(name='EventAccount')]})
+        # this should be modified to reflect LumiAlgsConf (configured separately?)
+
+    return fsrAlgs
 
 
 def configured_FunTuple(config):
diff --git a/Phys/DaVinci/python/DaVinci/config.py b/Phys/DaVinci/python/DaVinci/config.py
index ede4e9894..ad038c48b 100644
--- a/Phys/DaVinci/python/DaVinci/config.py
+++ b/Phys/DaVinci/python/DaVinci/config.py
@@ -18,8 +18,7 @@ from Configurables import ApplicationMgr
 from PyConf.application import configure, configure_input, configured_ann_svc
 from PyConf.application import metainfo_repos
 from PyConf.control_flow import CompositeNode, NodeLogic
-from PyConf.Algorithms import EventAccounting
-from DaVinci.algorithms import (define_fsr_writer, apply_filters)
+from DaVinci.algorithms import (make_fsr_algs, apply_filters)
 
 log = logging.getLogger(__name__)
 
@@ -176,15 +175,7 @@ def add_davinci_configurables(options, user_algorithms, public_tools):
 
     dvMainFlow = apply_filters(options, user_algorithms)
 
-    fsrAlgs = {}
-    if options.write_fsr and options.output_file:
-        if options.simulation:
-            fsrAlgs.update({"GenFSR": define_fsr_writer(options)})
-
-        if options.lumi:
-            fsrAlgs.update({"Lumi": [EventAccounting(name='EventAccount')]})
-            # this should be modified to reflect LumiAlgsConf (configured separately?)
-
+    fsrAlgs = make_fsr_algs(options)
     dvMainNode = davinci_control_flow(options,
                                       prepare_davinci_nodes(dvMainFlow),
                                       prepare_davinci_nodes(fsrAlgs))
diff --git a/Phys/DaVinci/tests/config/test_algorithms.py b/Phys/DaVinci/tests/config/test_algorithms.py
index 268670c57..36b72d877 100644
--- a/Phys/DaVinci/tests/config/test_algorithms.py
+++ b/Phys/DaVinci/tests/config/test_algorithms.py
@@ -11,7 +11,7 @@
 from PyConf.Algorithms import Gaudi__Examples__VoidConsumer as VoidConsumer
 
 from DaVinci import Options
-from DaVinci.algorithms import (define_fsr_writer, add_filter, apply_filters,
+from DaVinci.algorithms import (make_fsr_algs, add_filter, apply_filters,
                                 configured_FunTuple)
 from PyConf.reading import get_odin, get_decreports, get_hlt_reports, upfront_decoder
 from PyConf.application import default_raw_event
@@ -31,7 +31,7 @@ def test_define_write_fsr():
         merge_genfsr=True,
         simulation=True,
     )
-    test_algs = define_fsr_writer(options)
+    test_algs = make_fsr_algs(options)["GenFSR"]
     assert any("GenFSRMerge" == x.name for x in test_algs)
 
 
-- 
GitLab