get rid of existsAs in (some) tracking code #13062

mtosi · 2016-01-25T21:22:50Z

this PR should be almost identical to #12966
which can be close, indeed

I see that it cannot be automatically merge :(
how could I figure it out which is the source of the problem ?
thanks

cmsbuild · 2016-01-25T21:23:12Z

A new Pull Request was created by @mtosi (mia tosi) for CMSSW_8_0_X.

It involves the following packages:

HLTrigger/Configuration
RecoEgamma/EgammaElectronProducers
RecoEgamma/EgammaPhotonProducers
RecoHI/HiTracking
RecoLocalTracker/SiStripClusterizer
RecoTracker/ConversionSeedGenerators
RecoTracker/FinalTrackSelectors
RecoTracker/MeasurementDet
RecoTracker/TkTrackingRegions
TrackingTools/GsfTracking
TrackingTools/KalmanUpdators
TrackingTools/TrajectoryFiltering

@perrotta, @cmsbuild, @cvuosalo, @slava77, @Martin-Grunewald, @fwyzard, @davidlange6 can you please review it and eventually sign? Thanks.
@ghellwig, @forthommel, @abbiendi, @mandrenguyen, @nickmccoll, @lgray, @threus, @geoff-smith, @battibass, @makortel, @jhgoh, @dgulhan, @jlagram, @OlivierBondu, @yduhm, @yetkinyilmaz, @Sam-Harper, @GiacomoSguazzoni, @rovere, @VinInn, @bellan, @jalimena, @cerati, @mschrode, @richard-cms, @echapon, @trocino, @gbenelli, @jazzitup, @yenjie, @Martin-Grunewald, @kurtejung, @gpetruc, @istaslis this is something you requested to watch as well.
@slava77, @Degano, @smuzaffar you are the release manager for this.

cms-bot commands are list here #13028

slava77 · 2016-01-25T21:37:32Z

@mtosi
my guess is that the conflicts are with #13037
you can either wait for the next IB
or rebase your work to CMSSW_8_0_X

Martin-Grunewald · 2016-01-26T05:27:26Z

@mtosi
If this one replaces #12966 then YOU should actually close that PR!

Martin-Grunewald · 2016-01-26T05:40:31Z

@mtosi

Please add a comment in the file customizeHLTfor2016trackingTemplate
and tell what it is used for and what it is doing in somewhat more detail.

Martin-Grunewald · 2016-01-26T08:19:44Z

HLTrigger/Configuration/python/customizeHLTforCMSSW.py

@@ -44,6 +44,40 @@ def customiseFor12387(process):
            setattr(producer,'LegacyUnpacker',cms.bool(False))
    return process

+
+def customiseFor12966(process):


PR number now has changed (why didn't you UPDATE the old PR????)

cmsbuild · 2016-01-26T10:22:12Z

Pull request #13062 was updated. @perrotta, @cmsbuild, @cvuosalo, @slava77, @Martin-Grunewald, @fwyzard, @davidlange6 can you please check and sign again.

cmsbuild · 2016-01-26T10:48:32Z

Pull request #13062 was updated. @perrotta, @cmsbuild, @cvuosalo, @slava77, @Martin-Grunewald, @fwyzard, @davidlange6 can you please check and sign again.

cmsbuild · 2016-01-26T11:15:43Z

Pull request #13062 was updated. @perrotta, @cmsbuild, @cvuosalo, @slava77, @Martin-Grunewald, @fwyzard, @davidlange6 can you please check and sign again.

cmsbuild · 2016-01-27T22:41:07Z

Pull request #13062 was updated. @perrotta, @cmsbuild, @cvuosalo, @slava77, @Martin-Grunewald, @fwyzard, @davidlange6 can you please check and sign again.

… cfg file :(

cmsbuild · 2016-01-28T15:50:27Z

Pull request #13062 was updated. @perrotta, @cmsbuild, @cvuosalo, @slava77, @Martin-Grunewald, @fwyzard, @davidlange6 can you please check and sign again.

mtosi · 2016-01-28T15:53:48Z

Martin-Grunewald · 2016-01-28T16:59:11Z

I add the e-mail exchange here in the github PR to have a record.
Andrea Bocci's questions and their answers:

> 1. if #13062 contains all the changes from #13055, then close #13055
>
> 2. is customiseFor2016trackingTemplate() supposed to be called after the
> customisation in customizeHLTforCMSSW, or instead of it ? The former
should
> be OK, the latter would be a problem.
>
>
#13055 is merged, now

in order to have the tracking template in place
I added the following lines on top of the menu available in the release

# standard customization (which includes PR #13062 from tracking@HLT)
from HLTrigger.Configuration.customizeHLTforALL import customizeHLTforAll
process = customizeHLTforAll(process,_customInfo)

# tracking template for 2016
from HLTrigger.Configuration.customizeHLTfor2016trackingTemplate import
customiseFor2016trackingTemplate
process = customiseFor2016trackingTemplate(process)

open('dump.py','w').write(process.dumpPython())

hope it is fine
  mia

Martin-Grunewald · 2016-01-28T16:59:58Z

I add the e-mail exchange here in the github PR to have a record.
Martin's questions and their answers:

> It is also not clear to me what is needed when, and whether added
> parameters
> need to be able to be changed by POG/PAG testers or could remain "hidden"
> in customisation files until we parse pre6.
>
> needed parameters hidden in the customization file
should be available in the pre6 confDB templates
they should be handled by the fillDescription
and if it is not the case, because they are global PSet,
I have to add them
indeed, I would ask you to ping me when the confDB templates will be
available (thanks)


> It is also not clear whu you have both a modifcation in the std.
> custom file, AND you have added another custom file.

I change the standard one because I want to make sure that the standard
menu (2015) is not affected at all
the std customization is aimed to give you 2015 tracking

the new customization function is meant to provide the 2016 tracking while
the confDB templates and settings
(indeed I have anyhow to propagate the changes in confDB as well)
will be avialble
I did in this way because I understood other POGs and PAGs depend on this
changes and I do not want to increase the delay,
which unfortunately I already generated (even if it was not intended, of
course ... I had silly bugs to deal w/ and I'm still in the turn-on of
having a kid for the first time, I would say)

hope the code is fine and helpful
but, please, let me know if there are still issues/missing points
thanks
  mia

Martin-Grunewald · 2016-01-28T17:02:46Z

HLTrigger/Configuration/python/customizeHLTfor2016trackingTemplate.py

+
+        iter0seq = getattr(process,'HLTIterativeTrackingIteration0')
+        iter0seq.insert( iter0seq.index( iter0HP ), getattr(process,'hltIter0PFlowTrackCutClassifier') )
+        print 'iter0seq: ', iter0seq


Please remove this and all other "print" statements in the py files.
The printouts would end up in the final py file and lead to error as they are not valid python.
In any case the py file should be "silent"

cmsbuild · 2016-01-28T18:19:08Z

Pull request #13062 was updated. @perrotta, @cmsbuild, @cvuosalo, @slava77, @Martin-Grunewald, @fwyzard, @davidlange6 can you please check and sign again.

mtosi · 2016-01-28T18:19:17Z

thanks martin for catching my "dirty stuff" !

Martin-Grunewald · 2016-01-29T12:00:05Z

> I change the standard one because I want to make sure that the standard
> menu (2015) is not affected at all
> the std customization is aimed to give you 2015 tracking
>

Just to clarify my point of view: we are *not* keeping a 2015-like menu
alive in CMSSW 8.x anyway, so I think that we do not need to keep the 2015
settings as the default, once the 2016 settings are approved.

Once the 2016 menus are done and frozen, it will be a different story - but
right now I think we are free to make any change we want.

So the current modification to the std. customisation file is then only
to make the menu work with the new C++ code (and until we parse it into ConfDB).
Likewise for the added file and the 2016 tracking?
So can't we simply go directly to the 2016 tracking in the std. customisation file?

fwyzard · 2016-01-29T12:09:15Z

So can't we simply go directly to the 2016 tracking in the std.
customisation file?

Yes, that's what I was trying to say on Wednesday...

Martin-Grunewald · 2016-01-29T12:15:25Z

OK, then let's please do so:
remove the "new" custom file, and put the 2016 tk config in the "std" custom file.

cmsbuild · 2016-01-29T13:18:47Z

Pull request #13062 was updated. @perrotta, @cmsbuild, @cvuosalo, @slava77, @Martin-Grunewald, @fwyzard, @davidlange6 can you please check and sign again.

mtosi · 2016-01-29T13:20:18Z

added the customiseFor2016trackingTemplate in the standard one

some of the settings in the standard one are anyhow needed because some modules are used by other POGs

Martin-Grunewald · 2016-01-29T13:29:47Z

HLTrigger/Configuration/python/customizeHLTfor2016trackingTemplate.py

+    return process
+
+def customiseFor2016trackingTemplate(process):
+    bug_fix(process)


Here and in the following for all calls, you need to write process = bug_fix(process) etc.

slava77 · 2016-01-29T13:29:56Z

merge conflicts ...

Martin-Grunewald · 2016-01-29T13:30:18Z

HLTrigger/Configuration/python/customizeHLTforCMSSW.py.py

@@ -0,0 +1,69 @@
+import FWCore.ParameterSet.Config as cms


Please remove this file as a duplicate with a wrong name anyway: customizeHLTforCMSSW.py.py

cmsbuild · 2016-01-29T16:00:19Z

Pull request #13062 was updated. @perrotta, @cmsbuild, @cvuosalo, @slava77, @Martin-Grunewald, @fwyzard, @davidlange6 can you please check and sign again.

Martin-Grunewald · 2016-02-01T08:20:28Z

According to github, there are still "merge conflicts"; indeed:

Auto-merging HLTrigger/Configuration/python/customizeHLTforCMSSW.py
CONFLICT (content): Merge conflict in HLTrigger/Configuration/python/customizeHLTforCMSSW.py
Automatic merge failed; fix conflicts and then commit the result.

cmsbuild · 2016-02-01T12:18:18Z

Pull request #13062 was updated. @perrotta, @cmsbuild, @cvuosalo, @slava77, @Martin-Grunewald, @fwyzard, @davidlange6 can you please check and sign again.

mtosi · 2016-02-01T12:20:18Z

I thought I fixed the conflicts
I do not understand why it is still complaining ....
I've just re-fixed the file, but still it complains

I really do not understand :(

Martin-Grunewald · 2016-02-01T13:35:20Z

The error is still the same:

Auto-merging HLTrigger/Configuration/python/customizeHLTforCMSSW.py
CONFLICT (content): Merge conflict in HLTrigger/Configuration/python/customizeHLTforCMSSW.py
Automatic merge failed; fix conflicts and then commit the result.
Unable to merge branch refs/pull/13062/head from repository cms-sw.

Did you follow the suggested remedy?

Automatic merge failed; fix conflicts and then commit the result.

?

cmsbuild · 2016-02-02T09:45:42Z

Pull request #13062 was updated. @perrotta, @cmsbuild, @cvuosalo, @slava77, @Martin-Grunewald, @fwyzard, @davidlange6 can you please check and sign again.

cmsbuild · 2016-02-02T09:48:32Z

Pull request #13062 was updated. @perrotta, @cmsbuild, @cvuosalo, @slava77, @Martin-Grunewald, @fwyzard, @davidlange6 can you please check and sign again.

cmsbuild · 2016-02-02T11:51:47Z

Pull request #13062 was updated. @perrotta, @cmsbuild, @cvuosalo, @slava77, @Martin-Grunewald, @fwyzard, @davidlange6 can you please check and sign again.

Martin-Grunewald · 2016-02-02T13:19:31Z

@mtosi
This is getting us nowhere, the PR is still unmergable. I have redone the PR, see: #13164.
Please check(!!) and eventually sign and close this unmergeable one.

TRK PR #13062 redone (bug fixes, typos, remove existsAs etc.) to make it mergable (80X)

mtosi added 3 commits January 25, 2016 16:30

fix handling of exponential parameter (needed @HLT)

4c03c67

fix exist as and customizeHLTforCMSSW

10e227e

add customizeHLTfor2016trackingTemplate

34c7dbe

cmsbuild added this to the Next CMSSW_8_0_X milestone Jan 25, 2016

cmsbuild added reconstruction-pending hlt-pending pending-signatures tests-pending orp-pending comparison-pending labels Jan 25, 2016

Martin-Grunewald reviewed Jan 26, 2016
View reviewed changes

update PR name

e5c0fd3

fix merging issue

e91ac4b

add comment and make the customization function modular

cf6fd24

fix typo

399424c

fix handling of few modules which were not propagated properly in the…

42fdcbe

… cfg file :(

Martin-Grunewald reviewed Jan 28, 2016
View reviewed changes

clean print

c826298

add tracking 2016 template to standard customization

d9fd5a3

Martin-Grunewald reviewed Jan 29, 2016
View reviewed changes

fix

06c6596

fix merging issue

38aec07

clean

9002204

make use of tracking 2016 template

289a30d

fix

10c7e1d

Martin-Grunewald mentioned this pull request Feb 2, 2016

TRK PR #13062 redone (bug fixes, typos, remove existsAs etc.) to make it mergable (80X) #13164

Merged

davidlange6 closed this Feb 2, 2016

cmsbuild added a commit that referenced this pull request Feb 4, 2016

Merge pull request #13164 from Martin-Grunewald/MiaTosiFix

77cf558

TRK PR #13062 redone (bug fixes, typos, remove existsAs etc.) to make it mergable (80X)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

get rid of existsAs in (some) tracking code #13062

get rid of existsAs in (some) tracking code #13062

mtosi commented Jan 25, 2016

cmsbuild commented Jan 25, 2016

slava77 commented Jan 25, 2016

Martin-Grunewald commented Jan 26, 2016

Martin-Grunewald commented Jan 26, 2016

Martin-Grunewald Jan 26, 2016

cmsbuild commented Jan 26, 2016

cmsbuild commented Jan 26, 2016

cmsbuild commented Jan 26, 2016

cmsbuild commented Jan 27, 2016

cmsbuild commented Jan 28, 2016

mtosi commented Jan 28, 2016

Martin-Grunewald commented Jan 28, 2016

Martin-Grunewald commented Jan 28, 2016

Martin-Grunewald Jan 28, 2016

cmsbuild commented Jan 28, 2016

mtosi commented Jan 28, 2016

Martin-Grunewald commented Jan 29, 2016

fwyzard commented Jan 29, 2016

Martin-Grunewald commented Jan 29, 2016

cmsbuild commented Jan 29, 2016

mtosi commented Jan 29, 2016

Martin-Grunewald Jan 29, 2016

slava77 commented Jan 29, 2016

Martin-Grunewald Jan 29, 2016

cmsbuild commented Jan 29, 2016

Martin-Grunewald commented Feb 1, 2016

cmsbuild commented Feb 1, 2016

mtosi commented Feb 1, 2016

Martin-Grunewald commented Feb 1, 2016

cmsbuild commented Feb 2, 2016

cmsbuild commented Feb 2, 2016

cmsbuild commented Feb 2, 2016

Martin-Grunewald commented Feb 2, 2016

get rid of existsAs in (some) tracking code #13062

get rid of existsAs in (some) tracking code #13062

Conversation

mtosi commented Jan 25, 2016

cmsbuild commented Jan 25, 2016

slava77 commented Jan 25, 2016

Martin-Grunewald commented Jan 26, 2016

Martin-Grunewald commented Jan 26, 2016

Martin-Grunewald Jan 26, 2016

Choose a reason for hiding this comment

cmsbuild commented Jan 26, 2016

cmsbuild commented Jan 26, 2016

cmsbuild commented Jan 26, 2016

cmsbuild commented Jan 27, 2016

cmsbuild commented Jan 28, 2016

mtosi commented Jan 28, 2016

Martin-Grunewald commented Jan 28, 2016

Martin-Grunewald commented Jan 28, 2016

Martin-Grunewald Jan 28, 2016

Choose a reason for hiding this comment

cmsbuild commented Jan 28, 2016

mtosi commented Jan 28, 2016

Martin-Grunewald commented Jan 29, 2016

fwyzard commented Jan 29, 2016

Martin-Grunewald commented Jan 29, 2016

cmsbuild commented Jan 29, 2016

mtosi commented Jan 29, 2016

Martin-Grunewald Jan 29, 2016

Choose a reason for hiding this comment

slava77 commented Jan 29, 2016

Martin-Grunewald Jan 29, 2016

Choose a reason for hiding this comment

cmsbuild commented Jan 29, 2016

Martin-Grunewald commented Feb 1, 2016

cmsbuild commented Feb 1, 2016

mtosi commented Feb 1, 2016

Martin-Grunewald commented Feb 1, 2016

cmsbuild commented Feb 2, 2016

cmsbuild commented Feb 2, 2016

cmsbuild commented Feb 2, 2016

Martin-Grunewald commented Feb 2, 2016