fixed digi search behavior to check for end of list #12037
Conversation
|
A new Pull Request was created by @hardenbr for CMSSW_7_4_X. fixed digi search behavior to check for end of list It involves the following packages: HLTrigger/special @Martin-Grunewald, @perrotta, @cmsbuild, @fwyzard can you please review it and eventually sign? Thanks. |
|
Just to make sure: this is a genuine correction - or does the occurence of "end" reveal a mistake elsewhere and this is just a fix? |
| @@ -140,14 +140,17 @@ HLTRechitsToDigis::produce(edm::Event& iEvent, edm::EventSetup const& setup) { | |||
| // loop over the collection of rechits and match to digis | |||
| EcalRecHitCollection::const_iterator ituneEB; | |||
| for (ituneEB = recHitsHandle->begin(); ituneEB != recHitsHandle->end(); ituneEB++) { | |||
| EcalRecHit hit = (*ituneEB); | |||
| outputEBDigiCollection->push_back( (*digisEB->find(hit.id())).id(), (*digisEB->find(hit.id())).begin() ); | |||
| EcalRecHit hit = (*ituneEB); | |||
There was a problem hiding this comment.
could you change this to
EcalRecHit const & hit = * ituneEB;
to avoid the copy ?
(though hopefully the compiler optimises it out itself)
|
Pull request #12037 was updated. @Martin-Grunewald, @perrotta, @cmsbuild, @fwyzard can you please check and sign again. |
|
please test |
|
The tests are being triggered in jenkins. |
|
Pull request #12037 was updated. @Martin-Grunewald, @perrotta, @fwyzard can you please check and sign again. |
|
The jenkins tests job failed, please try again. |
|
@Martin-Grunewald This is being looked into. At some point a rechit that does not have a corresponding DIGI is being created and passed to the RechitsToDigi module. This happens in a very small fraction of events which is why it wasn't found until it was put online. |
|
+1 |
|
please test |
|
The tests are being triggered in jenkins. |
cmsbuild
added
hlt-approved
fully-signed
and removed
hlt-pending
pending-signatures
labels
|
This pull request is fully signed and it will be integrated in one of the next CMSSW_7_4_X IBs after it passes the integration tests and once validation in the development release cycle CMSSW_7_6_X is complete. This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @Degano, @smuzaffar |
|
This pull request is fully signed and it will be integrated in one of the next CMSSW_7_4_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_7_6_X is complete. This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @Degano, @smuzaffar |
|
This pull request is fully signed and it will be integrated in one of the next CMSSW_7_4_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_7_6_X is complete. This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @Degano, @smuzaffar |
|
@Martin-Grunewald Does a PR need to be done for 80X? |
|
Currently we have auto-port forward from 76 to 80... |
|
Add ref to issue: #11982 |
fixed digi search behavior to check for end of list
| EcalRecHit const & hit = (*ituneEB); | ||
| EcalDigiCollection::const_iterator digiLookUp = digisEB->find(hit.id()); | ||
| // protect against a digi not existing | ||
| if( digiLookUp == digisEB->end()) continue; |
There was a problem hiding this comment.
Reviewing this PR now.... this check makes no sense, there are no recHits without a digi associated. The recHit should not be skipped as it is now.
|
@argiro @ferriff @emanueledimarco @ghezzi please have a look |
|
I think that either the origin of the ghost hits is found (better) or the hits have to be skipped (patchy, but OK). Because this is what we will do anyway offline with them. |
|
RecHits with no digis might be there for recovered dead channels. Can this be checked comparing a list of DetId for which there is a recHit but no digi to the channel status in the DB? |
No description provided.