Ordering issue in Task.replace() affecting dumpPython()

[makortel]

The python 3 unit test failure

Traceback (most recent call last):
  File "/cvmfs/cms-ib.cern.ch/nweek-02609/slc7_amd64_gcc820/cms/cmssw-patch/CMSSW_11_1_PY3_X_2019-12-31-2300/python/FWCore/ParameterSet/Config.py", line 2425, in testTask
    self.assertTrue(process.path21.dumpPython(PrintOptions()) == 'cms.Path(process.mproducer10+process.mproducer8+process.mproducer8+(process.mproducer8+process.mproducer9)+process.sequence3, cms.Task(), cms.Task(process.None, process.mproducer10), cms.Task(process.d, process.mesproducer, process.messource, process.mfilter, process.mproducer10, process.mproducer2, process.myTask6), process.myTask100, process.myTask5)\n')
AssertionError: False is not true

seems to be caused by a different order of Tasks whose replace() functions are called by this line

cmssw/FWCore/ParameterSet/python/Config.py

Line 2387 in a04e878

process.myTask5 = Task()

Essentially the nested task structure before the call is

process.myTask5 = cms.Task(process.d) # d is Service
process.myTask8 = cms.Task(process.myTask5, process.mproducer8)
process.myTask7 = cms.Task(process.myTask8)
process.myTask6 = cms.Task(process.myTask7)
process.myTask1 = cms.Task(process.myTask6, ...)

In the py2 build, this loop

cmssw/FWCore/ParameterSet/python/Config.py

Lines 1071 to 1074 in a04e878

	def _replaceInTasks(self, label, new):
	old = getattr(self,label)
	for task in six.itervalues(self.tasks):
	task.replace(old, new)

operates first on process.myTask8 and sets its value to be cms.Task(process.mproducer8). Then the loop operates on process.myTask1, and nothing needs to be done.

In py3 build the loop operates first on process.myTask1. The visitor proceeds up to myTask8, removes the myTask5, and, because values were changed, recreates the nested task structure with anonymous Tasks

cmssw/FWCore/ParameterSet/python/SequenceTypes.py

Lines 1275 to 1280 in a04e878

	elif isinstance(visitee, Task):
	nonNull = []
	for c in contents:
	if c[0] is not None:
	nonNull.append(c[0])
	self.__stack[-2][-1] = [Task(*nonNull), True, None]

i.e. to

process.myTask1 = cms.Task(cms.Task(cms.Task(cms.Task(process.mproducer8, cms.Task()))), ...)

The anonymous Tasks lead to the failure of the test that expects to see process.myTask6 in the dumpPython() output. As far as I can tell, the issue affects only the dumpPython(), the module/component contents of the tasks are still correct.

[cmsbuild]

A new Issue was created by @makortel Matti Kortelainen.

@Dr15Jones, @smuzaffar, @silviodonato, @kpedro88, @davidlange6, @fabiocos can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

[Dr15Jones]

assign core

[cmsbuild]

New categories assigned: core

@Dr15Jones,@smuzaffar you have been requested to review this Pull request/Issue and eventually sign? Thanks