LogicError when using dump_python config to run

[srimanob]

Testing workflow, i.e. 39434.75, all steps run fine. But when I try to dump_python from one of the step and use it to run, i.e.
cmsDriver.py step2 -s DIGI:pdigi_valid,L1TrackTrigger,L1,DIGI2RAW,HLT:@fake2 --conditions auto:phase2_realistic_T21 --datatier GEN-SIM-DIGI-RAW -n 10 --eventcontent FEVTDEBUGHLT --geometry Extended2026D88 --era Phase2C17I13M9 --python DigiTrigger_HLT75e33_2026D88.py -n 50 --no_exec --filein file:step1.root --fileout file:step2.root --nThreads 8 --dump_python

I get the LogicError,

----- Begin Fatal Exception 17-May-2022 13:00:08 CEST-----------------------
An exception of category 'LogicError' occurred while
   [0] Constructing the EventProcessor
Exception Message:
Duplicate Product Identifier 
The Framework requires a unique branch name for each product
which consists of four parts: a friendly class name, module label,
product instance name, and process name. A product has been
registered with a duplicate branch name. The most common way
to fix this error is to modify the product instance name in
one of the offending 'produces' function calls. Another fix
would be to delete one of them if they are for the same product.

    friendly class name = edmHLTPathStatus
    module label = digitisation_step
    product instance name = 
    process name = HLT

The following additional information is not used as part of
the unique branch identifier.

    branch types = Event  Event
    class name = edm::HLTPathStatus

Note that if the four parts of the branch name are the same,
then this error will occur even if the branch types differ!

----- End Fatal Exception -------------------------------------------------

[cmsbuild]

A new Issue was created by @srimanob Phat Srimanobhas.

@Dr15Jones, @perrotta, @dpiparo, @makortel, @smuzaffar, @qliphy can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

[Dr15Jones]

So I ran the command you gave and I see the following in the python

process.schedule = cms.Schedule(*[ process.digitisation_step, process.L1TrackTrigger_step, process.L1simulation_step, process.digi2raw_step, process.digitisation_step, process.L1TrackTrigger_step, process.L1simulation_step, process.digi2raw_step, process.HLTriggerFirstPath, process.HLT_Physics_v1, process.HLT_Random_v1, process.HLT_ZeroBias_v1, process.HLTriggerFinalPath, process.HLTAnalyzerEndpath, process.endjob_step, process.FEVTDEBUGHLToutput_step, process.endjob_step, process.FEVTDEBUGHLToutput_step ], tasks=[process.patAlgosToolsTask])

If I just use the python as generated by running runTheMatrix.py then for step2 I see

# Schedule definition
# process.schedule imported from cff in HLTrigger.Configuration
process.schedule.insert(0, process.digitisation_step)
process.schedule.insert(1, process.L1TrackTrigger_step)
process.schedule.insert(2, process.L1simulation_step)
process.schedule.insert(3, process.digi2raw_step)
process.schedule.extend([process.endjob_step,process.FEVTDEBUGHLToutput_step])

If I interactively load the configuration for step2 generated by runTheMatrix.py and do process.dumpPython() then the schedule segment is

process.schedule = cms.Schedule(*[ process.digitisation_step, process.L1TrackTrigger_step, process.L1simulation_step, process.digi2raw_step, process.HLTriggerFirstPath, process.HLT_Physics_v1, process.HLT_Random_v1, process.HLT_ZeroBias_v1, process.HLTriggerFinalPath, process.HLTAnalyzerEndpath, process.endjob_step, process.FEVTDEBUGHLToutput_step ], tasks=[process.patAlgosToolsTask])

which does NOT show multiple process.digitisation_step, process.L1TrackTrigger_step, process.L1simulation_step and process.digi2raw_step in the Schedule.

It looks like cmsDriver.py's --dump_python command is calling the process.schedule.insert commands multiple times?

[makortel]

assign operations

It looks like cmsDriver.py's --dump_python command is calling the process.schedule.insert commands multiple times?

I was coming to the same conclusion.

[cmsbuild]

New categories assigned: operations

@fabiocos,@qliphy,@davidlange6,@perrotta you have been requested to review this Pull request/Issue and eventually sign? Thanks

[fabiocos]

it looks like the problem happens in https://github.com/cms-sw/cmssw/blame/master/Configuration/Applications/python/ConfigBuilder.py#L2260 , commenting this line removes the duplication. Of course this does not mean it is the solution, the issue is likely due to the previous lines added in #35858 .

[pkalbhor]

Faced similar issue during test of alca validation workflows. Raised a PR to address this issue PR#39127.
Expanding imports via exec and process.dumpPython() in same code where the instance of ConfigBuilder class is created is problematic.
Apparently fix is to expand imports in a sub-shell, which is why edmConfigDump gives correct result even though below lines from cmsDriver.py performs exactly same operation as edmConfigDump:

        if options.dump_python:
            result = {}
            exec(open(options.python_filename).read(), result)
            process = result["process"]
            expanded = process.dumpPython()
            expandedFile = open(options.python_filename,"w")
            expandedFile.write(expanded)
            expandedFile.close()

Replaced those lines with

        if options.dump_python:
            os.system('edmConfigDump -o {f} {f}'.format(f=options.python_filename))

[missirol]

Since this is somewhat related to #35858, I tried to understand the problem. My knowledge in python is basic, so the following is a bit vague.

I see that when exec is called in cmsDriver.py, the process.schedule object is None (as expected) up until the call

process.load('HLTrigger.Configuration.HLT_Fake2_cff')

but at the time of the exec call that import is cached in sys.modules, and after that load call via exec, somehow process.schedule will already include the full Schedule object [1], rather than the schedule defined by HLT_Fake2_cff [2]. This is why the ensuing calls to insert and extend in the cfg lead to duplicate entries in process.schedule.

[1] cms.Schedule(*[ process.digitisation_step, process.L1TrackTrigger_step, process.L1simulation_step, process.digi2raw_step, process.HLTriggerFirstPath, process.HLT_Physics_v1, process.HLT_Random_v1, process.HLT_ZeroBias_v1, process.HLTriggerFinalPath, process.HLTAnalyzerEndpath, process.endjob_step, process.FEVTDEBUGHLToutput_step ], tasks=[process.patAlgosToolsTask])
[2] cms.Schedule(*[ process.HLTriggerFirstPath, process.HLT_Physics_v1, process.HLT_Random_v1, process.HLT_ZeroBias_v1, process.HLTriggerFinalPath, process.HLTAnalyzerEndpath ])

A possible workaround illustrating the above is the following

diff --git a/Configuration/Applications/scripts/cmsDriver.py b/Configuration/Applications/scripts/cmsDriver.py
index a99a87a924f..24a69285436 100755
--- a/Configuration/Applications/scripts/cmsDriver.py
+++ b/Configuration/Applications/scripts/cmsDriver.py
@@ -33,6 +33,9 @@ def run():
 
         # handle different dump options
         if options.dump_python:
+            for cb_import in configBuilder.imports:
+              if cb_import in sys.modules:
+                del sys.modules[cb_import]
             result = {}
             exec(open(options.python_filename).read(), result)
             process = result["process"]

That said, I don't know what the best solution is.

[makortel]

If the problem indeed is that the ConfigBuilder ends up modifying the cms.Schedule object defined in the HLT_Fake2_cff python module, perhaps the ConfigBuilder should copy the Schedule object instead of modifying one defined elsewhere? (this is the general policy anyway for cff/cfi files, i.e. object should be modified only in the python modules that define them)

[missirol]

Agreed, and now it's clear that #35858 didn't comply with that policy.

I think it would look like this [*]. The copy in the output cfg itself, i.e.

"result += "process.schedule = process.schedule.copy()\n"

is not strictly necessary for the issue at hand, but I guess it'd be preferred? (given the policy)

[*]

diff --git a/Configuration/Applications/python/ConfigBuilder.py b/Configuration/Applications/python/ConfigBuilder.py
index 5397ed1505e..820890374c7 100644
--- a/Configuration/Applications/python/ConfigBuilder.py
+++ b/Configuration/Applications/python/ConfigBuilder.py
@@ -2250,13 +2250,20 @@ class ConfigBuilder(object):
             if not isinstance(self.scheduleIndexOfFirstHLTPath, int):
                 raise Exception('the schedule was imported from a cff in HLTrigger.Configuration, but the final index of the first HLT path is undefined')
 
+            # copy the schedule imported from the cff file in HLTrigger.Configuration
+            # in order not to modify directly the object defined inside the cff
+            self.process.schedule = self.process.schedule.copy()
+
+            result = "# process.schedule imported from cff in HLTrigger.Configuration\n"
+            result += "# copied to guarantee no changes to the schedule defined in the cff file\n"
+            result += "process.schedule = process.schedule.copy()\n"
+
             for index, item in enumerate(self.schedule):
                 if index < self.scheduleIndexOfFirstHLTPath:
                     self.process.schedule.insert(index, item)
                 else:
                     self.process.schedule.append(item)
 
-            result = "# process.schedule imported from cff in HLTrigger.Configuration\n"
             for index, item in enumerate(pathNames[:self.scheduleIndexOfFirstHLTPath]):
                 result += 'process.schedule.insert('+str(index)+', '+item+')\n'
             if self.scheduleIndexOfFirstHLTPath < len(pathNames):

[makortel]

The situation is somewhat convoluted. In a sense the ConfigBuilder is constructing a cfg file, for which the rules are more relaxed because the cfg file is supposed to be the main Python "program" (and therefore all modifications of the objects in the Process should be safe). I'd bet the Schedule is not the only object being modified in the ConfigBuilder-generated code that makes this assumption.

From this point of view the problem arises from ConfigBuilder having both in-memory version of the Process as well as textual representation, and then exec()'ng the textual representation, that leads to the objects being already loaded in the Process being used during the exec() call.

I suppose there is a choice to be made between treating ConfigBuilder as cfi/cff files regarding to the policy, or not loading the configuration again in memory for --dump_python (that is already done in #39127).

[srimanob]

What is the pros and cons to have dump_python the same as edmDumpConfig as proposed on #39127 ?

[pkalbhor]

I do not see any side effect of the proposed solution(#39127). It is compact and does not interfere with any module. Would love to see experts' opinion and resolve the issue with the choices proposed by @makortel.

[srimanob]

Done.