remove SIM customization from 8 TeV and postLS1 steps

[franzoni]

remove SIM customisation from 8 TeV and postLS1 steps => retrieve contextually from the Global Tag under the label 'Extended'

[cmsbuild]

A new Pull Request was created by @franzoni for CMSSW_7_1_X.

remove SIM customization from 8 TeV and postLS1 steps

It involves the following packages:

Configuration/PyReleaseValidation

@nclopezo, @vlimant, @cmsbuild, @franzoni, @Degano, @davidlange6 can you please review it and eventually sign? Thanks.
@ghellwig this is something you requested to watch as well.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
@nclopezo, @ktf you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

[davidlange6]

@franzoni - the GEN-SIM configuration is not control by just the geometry, so this is not in general a safe change - however, in this case the postls1customs is a no-op for GEN or SIM steps, so there is no change

On Apr 17, 2014, at 12:07 AM, franzoni notifications@github.com
wrote:

remove SIM customisation from 8 TeV and postLS1 steps => retrieve contextually from the Global Tag under the label 'Extended'

You can merge this Pull Request by running

git pull https://github.com/franzoni/cmssw from-CMSSW_7_1_X_2014-04-16-0200-sim-GEO-fromDB
Or view, comment on, or merge it at:

#3376

Commit Summary

• remove SIM customization from 8 TeV and postLS1 steps => retrived contextually from the Global Tag under the label 'Extended'
File Changes

• M Configuration/PyReleaseValidation/python/relval_steps.py (37)
Patch Links:

• https://github.com/cms-sw/cmssw/pull/3376.patch
• https://github.com/cms-sw/cmssw/pull/3376.diff
—
Reply to this email directly or view it on GitHub.

[franzoni]

+1
a full round of tests on standard and pile up is ok
@davidlange6 : I don't fully comprehend your remark. W/o any --geometry specified in the driver, the configuration points to the GT to retrieve the SIM geometry; this is the new policy, agreed/understood during this meeting https://indico.cern.ch/event/294004/ . Whatever manipulation was performed by customise functions when the SIM geometry was retrieved form xml, would be performed now too (there's actually no such customisation => easy, for today). And, following the logic of being fully GT-bound for geometry, I argue there never needs to be any geometry-related effect from customise functions: that just call for a modification of the relevant GT.

[cmsbuild]

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_1_X IBs unless changes or unless it breaks tests. @nclopezo, @ktf can you please take care of it?

[davidlange6]

On Apr 17, 2014, at 10:41 AM, franzoni notifications@github.com
wrote:

+1
a full round of tests on standard and pile up is ok
@davidlange6 : I don't fully comprehend your remark. W/o any --geometry specified in the driver, the configuration points to the GT to retrieve the SIM geometry; this is the new policy, agreed/understood during this meeting https://indico.cern.ch/event/294004/ . Whatever manipulation was performed by customise functions when the SIM geometry was retrieved form xml, would be performed now too (there's actually no such customisation => easy, for today). And, following the logic of being fully GT-bound for geometry, I argue there never needs to be any geometry-related effect from customise functions: that just call for a modification of the relevant GT.

Right. but the simulation application is not just a definition of a geometry... There was never a geometry customization. Conversely, had any geant4 changes been needed, they would be in the customization function. It was easy to have the same customization function for all steps so that no one has to think if the customization function is needed or not for any given step in the workflow.

But in any case, I don't think we have any sim level customizations aside from the gen validation (which is in any case a separate story from relvals)

—
Reply to this email directly or view it on GitHub.

[cmsbuild]

+1
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3376/938/summary.html

[cmsbuild]

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_1_X IBs unless changes (tests are also fine). @nclopezo, @ktf can you please take care of it?

[boudoul]

@davidlange6 & @franzoni : I am worried about this removal.... maybe for nothing, but... :
As David said, the custom is not related to the geometry policy (DB/xml) .. Still it would be better to check maybe a bit more what is actually does before removing it - And Indeed I found something which is potentially (?) not safe :
during the GENSIM step there is this customise functiun which is called :
process = customise_csc_PostLS1(process)
and it's easy to find what it does :
process = customise_csc_Geometry
defined here :
https://github.com/cms-sw/cmssw/blob/CMSSW_6_2_X_SLHC/SLHCUpgradeSimulations/Configuration/python/muonCustoms.py#L4
so maybe better to ask CSC experts before removing this customs without asking them ...

Also I don't think this is called ...
https://github.com/cms-sw/cmssw/blob/CMSSW_6_2_X_SLHC/SLHCUpgradeSimulations/Configuration/python/HCalCustoms.py#L102
but if it is called, then again removing the customs will impact...

So ... are you really sure you want to remove the postls1 customs without x-checking this ? (Again this has nothing to do with the DB/xml ...!!)

[davidlange6]

Hi Gaelle

On Apr 17, 2014, at 2:35 PM, boudoul notifications@github.com
wrote:

@davidlange6 & @franzoni : I am worried about this removal.... maybe for nothing, but... :
As David said, the custom is not related to the geometry policy (DB/xml) .. Still it would be better to check maybe a bit more what is actually does before removing it - And Indeed I found something which is potentially (?) not safe :
during the GENSIM step there is this customise functiun which is called :
process = customise_csc_PostLS1(process)
and it's easy to find what it does :
process = customise_csc_Geometry
defined here :
https://github.com/cms-sw/cmssw/blob/CMSSW_6_2_X_SLHC/SLHCUpgradeSimulations/Configuration/python/muonCustoms.py#L4
so maybe better to ask CSC experts before removing this customs without asking them ...

Also I don't think this is called ...
https://github.com/cms-sw/cmssw/blob/CMSSW_6_2_X_SLHC/SLHCUpgradeSimulations/Configuration/python/HCalCustoms.py#L102
but if it is called, then again removing the customs will impact...

So ... are you really sure you want to remove the postls1 customs without x-checking this ? (Again this has nothing to do with the DB/xml ...!!)

its ok - there is nothing in the top level customize function in postls1customs that calls anything if the only steps present are gen-sim. So by definition there is no effect.

Indeed, as for muonCustoms - it does alter the reco geometry in ways I was unaware of - will ask Yana to incorporate those changes in to the reco geometry. (which may or may not be trivial)

—
Reply to this email directly or view it on GitHub.

[davidlange6]

Hi again

@franzoni,
On second reading I am indeed incorrect. there are customization calls that are made in the gen-sim step and need to be checked by experts. I'm checking - but anyway, I would not remove the customize function for gen-sim yet.

On Apr 17, 2014, at 2:41 PM, David Lange David.Lange@cern.ch
wrote:

Hi Gaelle

On Apr 17, 2014, at 2:35 PM, boudoul notifications@github.com
wrote:

@davidlange6 & @franzoni : I am worried about this removal.... maybe for nothing, but... :
As David said, the custom is not related to the geometry policy (DB/xml) .. Still it would be better to check maybe a bit more what is actually does before removing it - And Indeed I found something which is potentially (?) not safe :
during the GENSIM step there is this customise functiun which is called :
process = customise_csc_PostLS1(process)
and it's easy to find what it does :
process = customise_csc_Geometry
defined here :
https://github.com/cms-sw/cmssw/blob/CMSSW_6_2_X_SLHC/SLHCUpgradeSimulations/Configuration/python/muonCustoms.py#L4
so maybe better to ask CSC experts before removing this customs without asking them ...

Also I don't think this is called ...
https://github.com/cms-sw/cmssw/blob/CMSSW_6_2_X_SLHC/SLHCUpgradeSimulations/Configuration/python/HCalCustoms.py#L102
but if it is called, then again removing the customs will impact...

So ... are you really sure you want to remove the postls1 customs without x-checking this ? (Again this has nothing to do with the DB/xml ...!!)

its ok - there is nothing in the top level customize function in postls1customs that calls anything if the only steps present are gen-sim. So by definition there is no effect.

Indeed, as for muonCustoms - it does alter the reco geometry in ways I was unaware of - will ask Yana to incorporate those changes in to the reco geometry. (which may or may not be trivial)

—
Reply to this email directly or view it on GitHub.

[franzoni]

@boudoul
Gaelle: I've not removed the customise function,

'--customise' : 'SLHCUpgradeSimulations/Configuration/postLS1Customs.customisePostLS1'

that's not on the table for now.
I've only changed the logic of retrieval of geometry: it was retrieved from the xml in the release, it's now from the correct record in the global tag.

[franzoni]

David, all,
I've not proposed that we remove the customise function now. The PR we're
discussing does not act on that.
In parallel, it's a good initiative if we do our best to remove the
customise function.
G.

On 17 April 2014 14:47, David Lange notifications@github.com wrote:

Hi again

@franzoni,
On second reading I am indeed incorrect. there are customization calls
that are made in the gen-sim step and need to be checked by experts. I'm
checking - but anyway, I would not remove the customize function for
gen-sim yet.

On Apr 17, 2014, at 2:41 PM, David Lange David.Lange@cern.ch
wrote:

Hi Gaelle

On Apr 17, 2014, at 2:35 PM, boudoul notifications@github.com
wrote:

@davidlange6 & @franzoni : I am worried about this removal.... maybe
for nothing, but... :
As David said, the custom is not related to the geometry policy
(DB/xml) .. Still it would be better to check maybe a bit more what is
actually does before removing it - And Indeed I found something which is
potentially (?) not safe :
during the GENSIM step there is this customise functiun which is called
:
process = customise_csc_PostLS1(process)
and it's easy to find what it does :
process = customise_csc_Geometry
defined here :

https://github.com/cms-sw/cmssw/blob/CMSSW_6_2_X_SLHC/SLHCUpgradeSimulations/Configuration/python/muonCustoms.py#L4
so maybe better to ask CSC experts before removing this customs without
asking them ...

Also I don't think this is called ...

https://github.com/cms-sw/cmssw/blob/CMSSW_6_2_X_SLHC/SLHCUpgradeSimulations/Configuration/python/HCalCustoms.py#L102
but if it is called, then again removing the customs will impact...

So ... are you really sure you want to remove the postls1 customs
without x-checking this ? (Again this has nothing to do with the DB/xml
...!!)

its ok - there is nothing in the top level customize function in
postls1customs that calls anything if the only steps present are gen-sim.
So by definition there is no effect.

Indeed, as for muonCustoms - it does alter the reco geometry in ways I
was unaware of - will ask Yana to incorporate those changes in to the reco
geometry. (which may or may not be trivial)

—
Reply to this email directly or view it on GitHub.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/3376#issuecomment-40709945
.

[boudoul]

ok guys, sorry I think I understodd the whole misundertanding ,
I think David and I both belived that you removed it , becuase of your initial comment " remove SIM customisation from 8 TeV and postLS1 steps " --> hence all the funny discussion between you ( " i don't undertand your remark " , "it was easy to have the same customization function for all steps" etc..)
and thn reading that I also thought you were removing the customise too ... so I have added entropy on top of the misundertanding.. a simple look at your file would have avoid this, so sorry for the noise , thanks!

[ianna]

@davidlange6 - I confirm, with conditions used --geometry option has to be dropped as @franzoni did in this PR. Yana

[cmsbuild]

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3376/938/summary.html

[cmsbuild]

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_1_X IBs unless changes (tests are also fine). @nclopezo, @ktf can you please take care of it?

[franzoni]

Hello Gaelle, David, ok - we're on the same ground now. The temporary
misunderstanding was a "provvida sventura" ( ~beneficial misfortune): the
discussion we've initiated about removing:

SLHCUpgradeSimulations/Configuration/postLS1Customs.customisePostLS1

is worth carrying on... if that can be done.
Cheers,
g.

On 17 April 2014 15:02, cmsbuild notifications@github.com wrote:

Comparison is ready

https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3376/938/summary.html

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/3376#issuecomment-40711141
.