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

Jump to bottom

New Ecal Local Reconstruction with Multifit #5197

Merged
merged 40 commits into from Sep 9, 2014
Merged

New Ecal Local Reconstruction with Multifit #5197

merged 40 commits into from Sep 9, 2014

Conversation

bendavid
Copy link
Contributor

@bendavid bendavid commented Sep 5, 2014

Adds additional EcalUncalibRecHitWorker for MultiFit (using Fast NNLS algorithm) and modifies appropriate sequences to activate it by default.

@argiro @emanueledimarco

bendavid and others added 30 commits August 31, 2014 01:44
@bendavid
Add (and activate) multifit reconstruction
6fed59e
@emanueledimarco
add the cfg starting from raw
602e95f
@emanueledimarco
- Add the 10 pulses amplitudes and uncertainties to the transient unc…
ff1836a 
…alibrated rechit.

- Remove the outof time energy and chi2, no more needed since the OOT energy is subtracted
@emanueledimarco
change the constents of the 13 bits reserved to OOT energy to the ene…
06932c1 
…rgy uncertainty from the multipulse fit
@emanueledimarco
change the uncalibrated rechit dataformats for the migration to the m…
5ff7e48 
…ultifit amplitude reco
@emanueledimarco
change the uncalibrated rechit dataformats for the migration to the m…
ea51885 
…ultifit amplitude reco
@emanueledimarco
change the container for the alternative amplitude reco after the mul…
e269455 
…tifit migration
@emanueledimarco
migration of the Ecal(Uncalib)RecHit dataformats to the new format af…
ee8913e 
…ter the multifit migration
@emanueledimarco
cleanup the cfg
37ad02f
@emanueledimarco
add some code to set the flags of kPoorReco and kOutOfTime (the secon…
86eaba7 
…d is not active at the moment)
@emanueledimarco
comment out the kPoorReco flag
b9a6bb3
@bendavid
migrate from minuit to inline TMatrix/TVector-based NNLS algorithm
55a061c
@bendavid
restore deprecated accessors to avoid interface incompatibility
3913455
@bendavid
fix indices for out of time energy in case not all bxs are active (mi…
5dce33e 
…ght happen in the future)
@bendavid
further performance optimizations
0a6ccaf
@bendavid
make minimizer a data member of the algo so that memory is only alloc…
f5d71ab 
…ated once
@emanueledimarco
move the inputs to the python files
6f36fd9
@emanueledimarco
Merge branch 'ecalMultifit_minimal720p4' of github.com:bendavid/cmssw…
e2ba69f 
… into ecalMultifit_minimal720p4

Merge the NNLS implementation with the DataFormats changes and the python configuration
@emanueledimarco
remove the uncertainties on the OOT pulses amplitudes
b46493e
@bendavid
add minos-like uncertainty computation
6a0fd2a
@bendavid
cleanup printout statements (and issue LogWarnings where appropriate)
005de8f
@emanueledimarco
bug fix on the OOT BXs amplitude assignement
c39179b
@emanueledimarco
change the time estimation with the weights, once subtracted the OOT PU
5c933d2
@bendavid
fix remaining small problem with bx indices
b6f46b3
@bendavid
pass set by const reference
8426f04
@bendavid
update pulse templates and covariance matrices
0dee3f1
@bendavid
fix input collections for default sequnce
491f225
@emanueledimarco
implement the weights for time reco, but turned off and configurable
7b25da1
@emanueledimarco
Merge branch 'ecalMultifit_minimal720p4' of github.com:bendavid/cmssw…
f96c361 
… into ecalMultifit_minimal720p4

Needed to add both the timing algorithm on pu-subtracted pulse shape and the newest complete covariance matrices from simulation
@bendavid
fix collection names in validation sequence
8a14d87
@vadler
Copy link

vadler commented Sep 9, 2014

@quertenmont : I added you to the thread simply to make you aware of the change.
A proper adoption to susybsm::HSCPCaloInfo would include the change of data members corresponding to that in the EcalRecHit, but this has to come with a separate PR by the SUSYBSM team after this one got integrated, if desired.

@quertenmont
Copy link
Contributor

@emanueledimarco
Well, our signal can be delayed by up to 50ns.
Our ECAL expert unfortunately left our group, so I can't judge myself what is meaningful or not, but I suspect that we are still interested by the method possibly biased by the 25ns PU.
I ofcourse don't have objection regarding this PR. I'd just like to understand how I can get similar behaviour as before (even if the definition is stlightly different).
Thanks in advance

@quertenmont
Copy link
Contributor

Thanks @vadler it's quite appreciated (it's just that it comes out from the blue, and I had not the time yet to go through the full conversion thread)

davidlange6 added a commit that referenced this pull request Sep 9, 2014
@davidlange6
Merge pull request #5197 from bendavid/ecalMultifit_minimal720p4
940e5d4 
New Ecal Local Reconstruction with Multifit
@davidlange6 davidlange6 merged commit 940e5d4 into cms-sw:CMSSW_7_2_X Sep 9, 2014
@lveldere
Copy link
Contributor

@bendavid @emanueledimarco @VinInn

Hi Josh and Emanuele

In CMSSW_7_2_X_2014-09-10-0200,
the first release with this pr included,
workflow 25401.0 (FatsSim) crashes.

Relevant lines of log
cmsRun: /build/cmsbuild/jenkins-workarea/workspace/ib-CMSSW_7_2_X-slc6_amd64_gcc481/243/w/tmp/BUILDROOT/55a55849db2d2c94176304e4e67c83bd/opt/cmssw/slc6_amd64_gcc481/cms/cmssw/CMSSW_7_2_X_2014-09-10-0200/
src/RecoTracker/CkfPattern/src/BaseCkfTrajectoryBuilder.cc:225: BaseCkfTrajectoryBuilder::StateAndLayers BaseCkfTrajectoryBuilder::findStateAndLayers(const TempTrajectory&) const: Assertion `!traj.empty(
)' failed.

The crash happens in HLT, in the module instance process.hltEgammaCkfTrackCandidatesForGSF.
Given the nature of the module, this is likely related to your pr.
Would you mind having a look?

The crash happens in architecture slc6_amd64_gcc481 but not in slc6_amd64_gcc491

Note:
in later IBs this crash is hidden by other crashes

Thanks a lot

Lukas

@bendavid
Copy link
Contributor Author

Hi Lukas,
Even if this issue is linked to the changes in the ecal reconstruction, the real problem here is surely somewhere in the tracking, which was not touched by this PR...

@lgray
Copy link
Contributor

lgray commented Sep 15, 2014

@lveldere Barring memory corruption I agree with Josh. Your more likely suspect is the CPE improvement (#5151) (if that's somehow made its way into HLT).

@lveldere
Copy link
Contributor

hm, 5151 is merged in a later IB
see https://cms-sw.github.io/showIB.html

@emanueledimarco
Copy link
Contributor

Ok, but still, it seems that it is related to some Ecal cluster that seeded a track in some weird region that made the ctf tracking crash. I don't see a crash in any ecal module

@StoyanStoynev StoyanStoynev mentioned this pull request Sep 17, 2014
Closed
bendavid pushed a commit to bendavid/cmssw that referenced this pull request Oct 13, 2014
@davidlange6 @bendavid
Merge pull request cms-sw#5197 from bendavid/ecalMultifit_minimal720p4
1e9d19e 
Backport of ecal multifit reconstruction to SLHC

New Ecal Local Reconstruction with Multifit
(Resolved) Conflicts:
	DQM/EcalMonitorTasks/python/CollectionTags_cfi.py
	DQM/EcalMonitorTasks/src/EnergyTask.cc
	DQM/Integration/python/test/ecal_dqm_sourceclient-live_cfg.py
	DQM/Integration/rcms/eb_dqm_sourceclient-live_cfg.py
	DQM/Integration/rcms/ee_dqm_sourceclient-live_cfg.py
	DataFormats/EcalRecHit/interface/EcalRecHit.h
	DataFormats/EcalRecHit/interface/EcalUncalibratedRecHit.h
	DataFormats/EcalRecHit/src/EcalUncalibratedRecHit.cc
	DataFormats/EcalRecHit/src/classes_def.xml
	DataFormats/EcalRecHit/test/testEcalRecHit.cppunit.cc
	RecoLocalCalo/Configuration/python/ecalLocalRecoSequence_cff.py
	RecoLocalCalo/EcalRecAlgos/BuildFile.xml
	RecoLocalCalo/EcalRecAlgos/interface/EcalRecHitSimpleAlgo.h
	RecoLocalCalo/EcalRecAlgos/src/ESRecHitSimAlgo.cc
	RecoLocalCalo/EcalRecProducers/test/plugins/EcalUncalibRecHitDump.cc
	RecoParticleFlow/PFClusterProducer/interface/PFEcalRecHitCreatorMaxSample.h
	SLHCUpgradeSimulations/Configuration/python/customise_phase1_hcal.py
	Validation/EcalRecHits/python/ecalRecHitsValidation_cfi.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
analysis-approved comparison-pending dqm-pending fastsim-pending new-package-pending operations-pending pending-signatures reconstruction-approved simulation-pending tests-approved
Projects
None yet
Milestone
Next CMSSW_7_2_X
Development

Successfully merging this pull request may close these issues.

None yet