Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop cause and ccctf #12576

Merged
merged 16 commits into from Dec 1, 2015
Merged

Stop cause and ccctf #12576

merged 16 commits into from Dec 1, 2015

Conversation

rovere
Copy link
Contributor

@rovere rovere commented Nov 26, 2015

Stop reason for a track/trajectory

Each TrajectoryFilter has been equipped in such a way to
inject into the Trajectory or TempTrajectory the reason that
stopped the propagation of the trajectory during the pattern
recognition. This work is in preparation of future work that
will propagate this information down the chain directly into
a reco::TrackBase, so that more detailed studies can be made
on the reasons of the stop.

CCC as a Trajectory Filter

The CCC has been implemented as a TrajectoryFilter, in order to give
more flexibility to the user in selecting possible bad (from the
Cluster Charge point of view) hits. The final trajectory, if discarded due
to eccessive number of bad CCC hits, will anyhow contain the
would-have-been removed 2 hits, since they passed all the other estimators.
The user has the possibility to disentangle lost hits from bad for CCC
and apply different criteria with different thresholds during Patter
Recognition and at filtering level.

NOTE
The CCC as Trajectory filter has been turned off, so that we should expect no changes and have this PR integrated quickly. The usage and tuning of this new features will be done on top of this. Private results are anyway extremely encouraging (w.r.t to the hit collection efficiency).

Each TrajectoryFilter has been equipped in such a way to
inject into the Trajectory or TempTrajectory the reason that
stopped the propagation of the trajectory during the pattern
recognition. This work is in preparation of future work that
will propagate this information down the chain directly into
a reco::TrackBase, so that more detailed studies can be made
on the reasons of the stop.
In order to better investigate the effects of various filters during
pattern recognition, a new data member has been added to the TrackBase
class. That, as a consecuence, triggered the increase in the
classVersion of its derived classes, which is also included into this
commit. Currently the information is stored an uint8_t in TrackBase,
but could be translated into a more proper enum class type as soon as
ROOT6 will digest it.
The logic of the filter has been wrongly changed(inverted) in a
previous commit. The correct behaviour is restored in this PR.
The main enum class has been moved to the DataFormats
folder, so that it is handy available to other DataFormats
products that need to use it. Also, every trajectory is now
initialized with the reserved value UNINITIALIZED, so that
we can better monitor all the cases in which the trajectory
has not been stopped by any filter being in any case
promoted to a track. All other stopping reasons have been
accordingly shifted.
The CCC has been implemented as a TrajectoryFilter, in order to give
more flexibility to the user in selecting possible bad (from the
Cluster Charge point of view) hits. The counter of the number of the
hits that would have been removed by the CCC if applied as an
estimator is left as a paramter, whose current values is 3 (i.e. we
tolerate up to 2 bad hits, after which we discard the trajectory). The
final trajectory, if discarded due to eccessive number of bad CCC
hits, will anyhow contain the would-have-been removed 2 hits, since
they passed all the other estimators. The user has the possibility to
disentangle lost hits from bad for CCC and apply different criteria
with different thresholds during Patter Recognition and at filtering
level. In order to assess the validity of this PR it would be better
to turn completely of the CCC at estimator level.
In the previous implementation, the counting of the hits that should
be considered as bad for CCC was done inside the filter itself, every
time looping on all TrajectoryMeasurements associated to the
Trajectory. With this improved implentation the counting is done
directly while propagating the Trajectory. The number of bad hits for
CCC is locally cached in the {Temp,}Trajectory and updated only with
the measurements of the appended segments.
Conflicts:
	TrackingTools/TrajectoryFiltering/interface/CkfBaseTrajectoryFilter.h
@cmsbuild
Copy link
Contributor

A new Pull Request was created by @rovere (Marco Rovere) for CMSSW_8_0_X.

It involves the following packages:

DataFormats/GsfTrackReco
DataFormats/SiStripCluster
DataFormats/TrackCandidate
DataFormats/TrackReco
RecoEgamma/EgammaPhotonProducers
RecoTracker/CkfPattern
RecoTracker/FinalTrackSelectors
RecoTracker/TrackProducer
TrackingTools/GsfTracking
TrackingTools/PatternTools
TrackingTools/TrajectoryFiltering

@cmsbuild, @cvuosalo, @davidlange6, @slava77 can you please review it and eventually sign? Thanks.
@ghellwig, @Sam-Harper, @battibass, @makortel, @threus, @abbiendi, @GiacomoSguazzoni, @jhgoh, @lgray, @bellan, @mschrode, @istaslis, @gpetruc, @cerati, @VinInn, @trocino, @dgulhan this is something you requested to watch as well.
@slava77, @Degano, @smuzaffar you are the release manager for this.

Following commands in first line of a comment are recognized

  • +1|approve[d]|sign[ed]: L1/L2's to approve it
  • -1|reject[ed]: L1/L2's to reject it
  • assign <category>[,<category>[,...]]: L1/L2's to request signatures from other categories
  • unassign <category>[,<category>[,...]]: L1/L2's to remove signatures from other categories
  • hold: L1/all L2's/release manager to mark it as on hold
  • unhold: L1/user who put this PR on hold
  • merge: L1/release managers to merge this request
  • [@cmsbuild,] please test: L1/L2 and selected users to start jenkins tests
  • [@cmsbuild,] please test with cms-sw/cmsdist#<PR>: L1/L2 and selected users to start jenkins tests using externals from cmsdist

@VinInn
Copy link
Contributor

VinInn commented Nov 26, 2015

@cmsbuild, please test

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/9962/console

@cmsbuild
Copy link
Contributor

-1
Tested at: 05cab46
When I ran the RelVals I found an error in the following worklfows:
134.911 step2

runTheMatrix-results/134.911_RunSinglePh2015D+RunSinglePh2015D+HLTDR2_25ns+RECODR2_25nsreHLT+HARVESTDR2_25nsreHLT/step2_RunSinglePh2015D+RunSinglePh2015D+HLTDR2_25ns+RECODR2_25nsreHLT+HARVESTDR2_25nsreHLT.log
----- Begin Fatal Exception 26-Nov-2015 18:07:36 CET-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing module: class=CkfTrajectoryMaker label='hltL3TrackCandidateFromL2OIState'
Exception Message:
MissingParameter: Parameter 'maxCCCLostHits' not found.
----- End Fatal Exception -------------------------------------------------

135.4 step1

runTheMatrix-results/135.4_ZEE_13+ZEEFS_13+HARVESTUP15FS+MINIAODMCUP15FS/step1_ZEE_13+ZEEFS_13+HARVESTUP15FS+MINIAODMCUP15FS.log
----- Begin Fatal Exception 26-Nov-2015 18:13:06 CET-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing module: class=CkfTrackCandidateMaker label='hltMu8Ele8CkfTrackCandidateMaker'
Exception Message:
MissingParameter: Parameter 'maxCCCLostHits' not found.
----- End Fatal Exception -------------------------------------------------

1306.0 step2

runTheMatrix-results/1306.0_SingleMuPt1_UP15+SingleMuPt1_UP15+DIGIUP15+RECOUP15+HARVESTUP15/step2_SingleMuPt1_UP15+SingleMuPt1_UP15+DIGIUP15+RECOUP15+HARVESTUP15.log
----- Begin Fatal Exception 26-Nov-2015 18:14:55 CET-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing module: class=CkfTrajectoryMaker label='hltL3TrackCandidateFromL2OIState'
Exception Message:
MissingParameter: Parameter 'maxCCCLostHits' not found.
----- End Fatal Exception -------------------------------------------------

1330.0 step2

runTheMatrix-results/1330.0_ZMM_13+ZMM_13+DIGIUP15+RECOUP15+HARVESTUP15/step2_ZMM_13+ZMM_13+DIGIUP15+RECOUP15+HARVESTUP15.log
----- Begin Fatal Exception 26-Nov-2015 18:19:55 CET-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing module: class=CkfTrajectoryMaker label='hltL3TrackCandidateFromL2OIState'
Exception Message:
MissingParameter: Parameter 'maxCCCLostHits' not found.
----- End Fatal Exception -------------------------------------------------

25202.0 step2

runTheMatrix-results/25202.0_TTbar_13+TTbar_13+DIGIUP15_PU25+RECOUP15_PU25+HARVESTUP15_PU25/step2_TTbar_13+TTbar_13+DIGIUP15_PU25+RECOUP15_PU25+HARVESTUP15_PU25.log
----- Begin Fatal Exception 26-Nov-2015 18:27:37 CET-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing module: class=CkfTrajectoryMaker label='hltL3TrackCandidateFromL2OIState'
Exception Message:
MissingParameter: Parameter 'maxCCCLostHits' not found.
----- End Fatal Exception -------------------------------------------------

50202.0 step2

runTheMatrix-results/50202.0_TTbar_13+TTbar_13+DIGIUP15_PU50+RECOUP15_PU50+HARVESTUP15_PU50/step2_TTbar_13+TTbar_13+DIGIUP15_PU50+RECOUP15_PU50+HARVESTUP15_PU50.log
----- Begin Fatal Exception 26-Nov-2015 18:29:39 CET-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing module: class=CkfTrajectoryMaker label='hltL3TrackCandidateFromL2OIState'
Exception Message:
MissingParameter: Parameter 'maxCCCLostHits' not found.
----- End Fatal Exception -------------------------------------------------

you can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-12576/9962/summary.html

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
79b1098
You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-12576/9962/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-12576/9962/git-merge-result


explicit MaxCCCLostHitsTrajectoryFilter (
const edm::ParameterSet & pset, edm::ConsumesCollector& iC) :
theMaxCCCLostHits_(pset.getParameter<int>("maxCCCLostHits")),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

protect these...

@slava77
Copy link
Contributor

slava77 commented Nov 26, 2015

please test (compile and run the short matrix) before updating the PR or requesting another jenkins tests.
There is a limited amount of resources and space for the tests and it can interfere with other activities.

* patter recognition. Used mainly as a criteria for abandoning a
* trajectory candidate during trajectory building.
*/
int CCCBadHits() const { return theNumberOfCCCBadHits_;}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as in TempTrajectory:
start with a lower case
cccBadHits is just as good
... in general, I find it a bad idea to define a category based on one particular cut. If we redo this cut to do something else (not just cluster charge), the name will become inappropriate.

@slava77
Copy link
Contributor

slava77 commented Nov 28, 2015

@rovere
please update the method names, after that this PR can be signed off quickly.
Thanks.

@slava77
Copy link
Contributor

slava77 commented Nov 30, 2015

@cmsbuild please test

@cmsbuild
Copy link
Contributor

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/10030/console

@cmsbuild
Copy link
Contributor

Pull request #12576 was updated. @cmsbuild, @cvuosalo, @davidlange6, @slava77 can you please check and sign again.

@slava77
Copy link
Contributor

slava77 commented Dec 1, 2015

+1

for #12576 265e83b

  • code changes are in line with the description
  • previous jenkins tests OK: passed and comparisons showed no differece; the last change is trivial (jenkins compiled OK)

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 1, 2015

This pull request is fully signed and it will be integrated in one of the next CMSSW_8_0_X IBs after it passes the integration tests. This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @Degano, @smuzaffar

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 1, 2015

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 1, 2015

@davidlange6
Copy link
Contributor

+1

cmsbuild added a commit that referenced this pull request Dec 1, 2015
@cmsbuild cmsbuild merged commit 15cc2e6 into cms-sw:CMSSW_8_0_X Dec 1, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants