Problems with Modifiers and multiple processes

[silviodonato]

In the context of the studies of the difference between GPU/CPU, we've found a conflict between the L1 emulator https://github.com/cms-sw/cmssw/blob/master/HLTrigger/Configuration/python/CustomConfigs.py#L123-L159 and the customizeHLTforPatatrack function https://github.com/cms-sw/cmssw/blob/master/HLTrigger/Configuration/python/customizeHLTforPatatrack.py#L148 .

You can very easily reproduce this problem with the following code

import FWCore.ParameterSet.Config as cms

process = cms.Process( "AAA" )

def foo(process):
    from Configuration.Eras.Era_Run3_cff import Run3
    tmp = cms.Process('BBB', Run3)
    return process

process = foo(process)

from Configuration.Eras.Modifier_run3_common_cff import run3_common

process.TestPSet = cms.PSet( 
  isRun2 = cms.bool( True ),
)

print("==================================================")
print("process.isUsingModifier(run3_common) ", process.isUsingModifier(run3_common))
print("process.TestPSet.isRun2 ", process.TestPSet.isRun2)
print("process.name_() ", process.name_())
print("**************************************************")
run3_common.toModify(process.TestPSet, isRun2 = False)
print("process.isUsingModifier(run3_common) ", process.isUsingModifier(run3_common))
print("process.TestPSet.isRun2 ", process.TestPSet.isRun2)
print("process.name_() ", process.name_())
print("==================================================")
print(str(globals()))

The outuput is

==================================================
process.isUsingModifier(run3_common)  False
process.TestPSet.isRun2  cms.bool(True)
process.name_()  AAA
**************************************************
process.isUsingModifier(run3_common)  False
process.TestPSet.isRun2  cms.bool(False)
process.name_()  AAA
==================================================
{'__name__': '__main__', '__doc__': None, '__package__': None, '__loader__': <_frozen_importlib_external.SourceFileLoader object at 0x7f6336c56c10>, '__spec__': None, '__annotations__': {}, '__builtins__': <module 'builtins' (built-in)>, '__file__': '/eos/home-s/sdonato/TSG/CMSSW_12_4_X_2022-04-26-2300/src/hlt_pixel.py', '__cached__': None, 'cms': <module 'FWCore.ParameterSet.Config' from '/cvmfs/cms-ib.cern.ch/week0/slc7_amd64_gcc10/cms/cmssw-patch/CMSSW_12_4_X_2022-04-26-2300/python/FWCore/ParameterSet/Config.py'>, 'process': <FWCore.ParameterSet.Config.Process object at 0x7f6336b7fa00>, 'foo': <function foo at 0x7f6336c2cb80>, 'run3_common': <FWCore.ParameterSet.Config.Modifier object at 0x7f632ee57d90>}

I expected that process.isUsingModifier would remain cms.bool(True).

[cmsbuild]

A new Issue was created by @silviodonato Silvio Donato.

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

cms-bot commands are listed here

[silviodonato]

cc @cms-sw/hlt-l2 @cms-sw/l1-l2 as it is related to https://github.com/cms-sw/cmssw/blob/master/HLTrigger/Configuration/python/CustomConfigs.py#L123-L159

[makortel]

assign core

[cmsbuild]

New categories assigned: core

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

[fwyzard]

I think there are two problems.

1. the constructor of the temporary cms.Process('BBB', Run3) object created inside foo calls Run3._setChosen(), which "enables" all the modifiers inside the Run3 era.
   Since modifiers are global objects, not attached to a specific Process, any customisation like run3_common.toModify(process.TestPSet, isRun2 = False) will see those modifiers as enabled, and apply the required change.

2. process.isUsingModifier(mod) does a different thing: if mod is enabled, it checks if any of the modifiers or eras attached to the process contains it.
   So, even if mod._isChosen() is True, in the example process.isUsingModifier(mod) returns False because process does not use it.

[fwyzard]

It looks like the behaviour of process.isUsingModifier is the intended one, at least from the method's name.

While the global nature of the modifiers is causing the unexpected behaviour :-(

[makortel]

This is an expected behavior with the current implementation of the Modifier. The enabled/disabled status of the Modifier is stored in the Modifier object itself (in this case run3_common, which is included in the Run3 ModifierChain). The Modifier object (run3_common) is the same for both cms.Process objects, and therefore as soon as anything enables the Modifier, it is enabled globally.

(mostly for future record) The underlying issue is faced also if trying to implement MiniAOD in the same (posix) process as RECO as a SubProcess, when MiniAOD and RECO would use different Modifiers, that was discussed in https://mattermost.web.cern.ch/cms-o-and-c/pl/6u8d7kdgmjfciguki4gchmicsy.

[makortel]

Why does

cmssw/HLTrigger/Configuration/python/CustomConfigs.py

Lines 123 to 127 in 12b3153

	def L1REPACK(process, sequence="Full"):
	from Configuration.Eras.Era_Run3_cff import Run3
	l1repack = cms.Process('L1REPACK', Run3)
	l1repack.load('Configuration.StandardSequences.SimL1EmulatorRepack_'+sequence+'_cff')

need to create a second Process object? Is the argument process not using Run3 modifier?

[fwyzard]

Is the argument process not using Run3 modifier?

Not necessarily... e.g. not when running over Run-2 data to validate the GPU-vs-CPU code behaviour 😞

[makortel]

Is the argument process not using Run3 modifier?

Not necessarily... e.g. not when running over Run-2 data to validate the GPU-vs-CPU code behaviour

Is using Run3 setup for this L1 configuration the right thing to do in such case(s)?

[Martin-Grunewald]

Yes, we want to use real collisions data (hence from Run2) but rerun using the Run3 L1T re-emulation.

[silviodonato]

Yes, because we want to emulate the latest L1 menu (ie. the Run3 menu) using Run2 data.
In order to use Run2 data, we need to the pixel thresholds to those used in Run2 and then we used the Run3 modifier https://github.com/cms-sw/cmssw/blob/master/HLTrigger/Configuration/python/customizeHLTforPatatrack.py#L148 .

Anyway, we can find a workaround for this specific case. The issue wanted to be more generic.
Do you think that also process.isUsingModifier(run3_common) has the expected behavior?

[makortel]

The issue wanted to be more generic.

I'm afraid we can't do much without significantly constraining or changing the configuration system, which would have lots of knock-on effects.

Do you think that also process.isUsingModifier(run3_common) has the expected behavior?

The behavior looks correct. Although given global nature of the selection state of the Modifers, I acknowledge its result can be confusing to interpret.

[fwyzard]

Hi Matti, while changing the current behaviour is likely quite cumbersome, maybe it would be simple enough to detect this kind of unwanted situations where different Process objects are created with a different sent of modifiers, and give some meaningful error message ? .A

[makortel]

while changing the current behaviour is likely quite cumbersome, maybe it would be simple enough to detect this kind of unwanted situations where different Process objects are created with a different sent of modifiers, and give some meaningful error message ?

Yes, a check and an error message should be possible to do.

[makortel]

For this particular case, would it be feasible to figure out the set of Modifiers actually being used in Configuration.StandardSequences.SimL1EmulatorRepack_*_cff (and cfi/cff fragments imported there), and pass those to the cms.Process() of the "main process" along the relevant Run2 era?

[fwyzard]

I've tried going through the imports of the L1 configuration, and after a few levels of import a non-exhaustive list includes (leaving out the ones dealing with Phase 2, heavy ions, and premixing)

ctpps
hcalSkipPacker
run2_DT_2018
run2_GEM_2017
run2_HCAL_2018
run2_HE_2017
run2_HF_2017
run2_common
run3_GEM
run3_HB
run3_RPC
run3_common
stage1L1Trigger
stage2L1Trigger
stage2L1Trigger_2017
stage2L1Trigger_2018
stage2L1Trigger_2021

Some are L1-specific, but others are tied to the detectors (likely because of the L1 trigger primitives), so I'm not sure they can be used in the top-level process.

[fwyzard]

Here's just an idea, assuming the current approach picks up the correct modifiers...

What if we extend

def L1REPACK(process, sequence="Full"):
    from Configuration.Eras.Era_Run3_cff import Run3
    l1repack = cms.Process('L1REPACK', Run3)
    l1repack.load('Configuration.StandardSequences.SimL1EmulatorRepack_'+sequence+'_cff')
    ...
    return process

with the possibility of setting and resetting modifiers explicitly ?

Something like

def L1REPACK(process, sequence="Full"):
    # make a copy of the modifiers active in the current Process
    activeModifiers = list(process.Process__modifiers)

    # reset all active modifiers
    for mod in process.Process__modifiers:
      mod.Modifier__chosen = False

    from Configuration.Eras.Era_Run3_cff import Run3
    l1repack = cms.Process('L1REPACK', Run3)
    l1repack.load('Configuration.StandardSequences.SimL1EmulatorRepack_'+sequence+'_cff')
    ...

    # reset all active modifiers
    for mod in l1repack.Process__modifiers:
      mod.Modifier__chosen = False

    # create a dummy Process to re-enable the modifiers that were originally active
    unused = cms.Process('UNUSED', *activeModifiers)
    return process

It is a hack, but it should provide the intended behaviour, hopefully without breaking anything else.

[makortel]

I can quickly think of one caveat with the hack, that is related to the load/import order of the python files. If all of the python files loaded during the "hacked state of Modifiers" are loaded only there (i.e. none of them are used before or after that snippet), I'd expect the hack to provide the intended behavior.

But if any of the concerned python files is loaded also before the hack, the python objects defined in that file have the state they got with the original active Modifiers. Their state does not change if such file is used later with different set of Modifiers being active.

Similarly, if any of the concerned python files is loaded after the hack, the python objects defined in that file have the state they got with the hacked active Modifiers.

[fwyzard]

I see - yes, you are right, even if we don't have to worry about concurrency here, the order in which the modules are loaded still matters, since modules are global and Python should not re-load them.

[makortel]

One option would be to make and use L1T-specific modifiers in the necessary files. For example, assuming the Run2_2018 would be the common base (and stage2L1Trigger_2021 is not used outside of this context, not sure how ctpps or hcalSkipPacker should be treated though) this could mean

run3_GEM_in_l1repack
run3_HB_in_l1repack
run3_RPC_in_l1repack
run3_common_in_l1repack

(names are only for illustration) Modifiers to be used in these files instead of run3_{GEM,HB,RPC,common}, and adding these modifiers to the Run3 Era. The necessary Run3-specific functionality could then be enabled along Run2_2018 Era in the main Process object by giving these *_l1repack Modifiers for the Process constructor (or one could define a specific ModifierChain for them for easier use).

[makortel]

while changing the current behaviour is likely quite cumbersome, maybe it would be simple enough to detect this kind of unwanted situations where different Process objects are created with a different sent of modifiers, and give some meaningful error message ?

Yes, a check and an error message should be possible to do.

This check is added in #37903

[silviodonato]

Thank you. I tried again the original test code in CMSSW_12_4_0_pre4 and I got

Traceback (most recent call last):
  File "/eos/home-s/sdonato/TSG/Wisconsin/CMSSW_12_4_0_pre4/src/test.py", line 10, in <module>
    process = foo(process)
  File "/eos/home-s/sdonato/TSG/Wisconsin/CMSSW_12_4_0_pre4/src/test.py", line 7, in foo
    tmp = cms.Process('BBB', Run3)
  File "/cvmfs/cms.cern.ch/slc7_amd64_gcc10/cms/cmssw/CMSSW_12_4_0_pre4/python/FWCore/ParameterSet/Config.py", line 156, in __init__
    raise RuntimeError("The Process {} tried to redefine which Modifiers to use after another Process was already started".format(name))
RuntimeError: The Process BBB tried to redefine which Modifiers to use after another Process was already started
[sdonato@sdonato-vm src]$ pwd
/afs/cern.ch/user/s/sdonato/TSG/Wisconsin/CMSSW_12_4_0_pre4/src

Closing this issue

[makortel]

+core

[cmsbuild]

This issue is fully signed and ready to be closed.