Consider extending Modifier.toReplaceWith to allow transforming functions

[Dr15Jones]

What if we had another form of toReplaceWith which takes a callable object. If the era is chosen, the sequence would be cloned then the clone passed to the callable object and finally the replace would occur based on the output of the callable object. That way eras could properly chain toReplaceWith. E.g.

def _run3_globalValidation(seq):
   global gemSimValid
   return seq+gemSimValid

def _phase2_globalValidation(seq):
   global gemSimValid
   seq = seq.copyAndExclude([gemSimValid])
   return seq+ me0SimValid

eras.run3_GEM.toReplaceWith( globalValidation, transform=_run3_globalValidation )
eras.phase2_muon.toReplaceWith(globalValidation, transform=_phase2_globalValidation)

[cmsbuild]

A new Issue was created by @Dr15Jones (Chris Jones).

@davidlange6, @smuzaffar, @degano, @davidlt, @Dr15Jones 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

[Dr15Jones]

Given that Modifier.toModify already has a syntax for passing functions, this proposal should also use a similar syntax. E.g.

def _func(seq):
   global foo
   seq += foo

eras.bar.toReplaceWith(fii, func = _func)

[kpedro88]

Personally, I would support this option to avoid colliding uses of toReplaceWith by multiple Eras.

[Dr15Jones]

@kpedro88 pointed out the better example is

def _run3_globalValidation(seq):
   global gemSimValid
   return seq+gemSimValid

def _phase2_globalValidation(seq):
   global me0SimValid
   return seq+ me0SimValid

eras.run3_GEM.toReplaceWith( globalValidation, transform=_run3_globalValidation )
eras.phase2_muon.toReplaceWith(globalValidation, transform=_phase2_globalValidation)

Which shows how the two eras properly co-exist.

[slava77]

def _run3_globalValidation(seq):
   global gemSimValid
   return seq+gemSimValid

def _phase2_globalValidation(seq):
   global gemSimValid
   seq = seq.copyAndExclude([gemSimValid])
   return seq+ me0SimValid

this example is rather impractical. The use case for this example was to not repeat the same modules/sequences in Run3 and Phase2 definitions for cases where Run3 edits should be applied to Phase2. ... imagine we need to write all phase2 modifiers starting with explicit removal of all run2 modifiers.

[Dr15Jones]

In order to avoid use of global it would be better to use lambda

eras.run3_GEM.toReplaceWith(globalValidation, func = lambda seq: return seq + gemSimValid)
eras.phase2_muon.toReplaceWith(globalValidation, func = lambda seq : return seq + me0SimValid)

[smuzaffar]

@Dr15Jones , can we close this?

[Dr15Jones]

@makortel thoughts on this?

[makortel]

Hmm. The current pattern corresponding to https://github.com/cms-sw/cmssw/issues/14276#issuecomment-215150322 seems to be

_run3_globalValidation = globalValidation.copy()
_run3_globalValidation += gemSimValid

_phase2_globalValidation = _run3_globalValidation.copy()
_phase2_globalValidation += me0SimValid

run3_GEM.toReplaceWith( globalValidation, _run3_globalValidation )
phase2_muon.toReplaceWith( globalValidation, _phase2_globalValidation )

In the spirit of https://github.com/cms-sw/cmssw/issues/14276#issuecomment-215150322 one could think of shortening it to

run3_GEM.toReplaceWith( globalValidation, globalValidation.copy()+gemSimValid )
phase2_muon.toReplaceWith( globalValidation, globalValidation.copy()+me0SimValid )

but that would actually change the behavior: currently phase2_muon includes gemSimValid regardless of run3_GEM, whereas in the shorthand above (and in the earlier comment) phase2_muon would include gemSimValid only if run3_GEM would be active as well. So the proper shorthand would be

run3_GEM.toReplaceWith( globalValidation, globalValidation.copy()+gemSimValid )
phase2_muon.toReplaceWith( globalValidation, globalValidation.copy()+gemSimValid+me0SimValid )

The need to repeat gemSimValid in the latter makes me disfavor it.

With the proposed functions the behavior could be achieved with e.g.

_run3_GEM_globalValidation = lambda seq: return seq + gemSimValid
run3_GEM.toReplaceWith(globalValidation, func = _run3_GEM_globalValidation)
phase2_muon.toReplaceWith(globalValidation: func = lambda seq: return _run3_GEM_globalValidation(seq) + me0SimValid)

I'm not really sure at the moment what would be the clearest option. With toModify() I've personally preferred the use of dict() as much as I can over the function variant.