SimHit vs. RecHit Validation #16734
SimHit vs. RecHit Validation #16734
Conversation
…lRecHits/HcalRecHitsValidation in PR # Support for testNumbering in the SimHit collection is added.
…alRecHits/interface/HcalRecHitsValidation.h fixed testNumbering customization in Validation/HcalRecHits/python/HcalRecHitParam_cfi.py standardized profile booking to remove unused y-axis binning
|
A new Pull Request was created by @kencall for CMSSW_9_0_X. It involves the following packages: Validation/HcalRecHits @cmsbuild, @dmitrijus, @vanbesien, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are listed here #13028 |
|
@cmsbuild please test |
|
The tests are being triggered in jenkins. |
|
Comparison job queued. |
|
+1 |
cmsbuild
added
dqm-approved
fully-signed
and removed
dqm-pending
pending-signatures
labels
|
This pull request is fully signed and it will be integrated in one of the next CMSSW_9_0_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @smuzaffar |
|
|
||
| if ( r < partR ){ // just energy in the small cone | ||
| enSimHits += en; | ||
| if(sub == static_cast<int>(HcalBarrel)) enSimHitsHB += en; |
There was a problem hiding this comment.
@kencall - can you clarify the difference between "sub" and "subdet_" in this module? thanks
There was a problem hiding this comment.
@davidlange6 it looks like subdet_ is used to control which histograms actually get filled by the analyzer. sub is the per-SimHit subdetector (all Hcal SimHits are grouped together in one collection).
There was a problem hiding this comment.
thats what I thought to - but then I would expect an if ( sub != subdet_ ) continue...re-rereading, maybe that is exactly what should be added..(and the 5 energy and cone variables could become 2..)?
There was a problem hiding this comment.
There could be some simplification - it would have to be (sub!=subdet_ and subdet_==5) continue, and there would still have to be a check for HF to fill the separate long and short energy deposits. I guess this could also be done when filling the RecHit variables like eHcalConeHB.
There was a problem hiding this comment.
HF has to be handled separately regardless since HF simhit energy is defined in p.e.'s and not GeV.
I think that it still makes sense to compare SimHit to RecHit energy for each subdetector, especially when their are different electronics in HB and HE.
There was a problem hiding this comment.
I think @davidlange6 is just saying that, in the case where subdet_ is set to a specific index, there's no need to fill variables for the other subdets.
There was a problem hiding this comment.
Ok, that makes sense -- especially since the values are never used if subdet_ is set to a specific detector.
|
Right - maybe as not to make this overly complex, lets add the continue statement and leave the rest of the code. Seems eventually its better to have one module that fills all histograms given the unnaturalness in the algorithm as it is..
… On Nov 29, 2016, at 7:16 PM, Kevin Pedro ***@***.***> wrote:
@kpedro88 commented on this pull request.
In Validation/HcalRecHits/src/HcalRecHitsValidation.cc:
> + cell = HcalDetId(SimHits->id());
+ sub = cell.subdet();
+ depth = cell.depth();
+ }
+
+ const CaloCellGeometry* cellGeometry =
+ geometry->getSubdetectorGeometry (cell)->getGeometry (cell) ;
+ double etaS = cellGeometry->getPosition().eta () ;
+ double phiS = cellGeometry->getPosition().phi () ;
+ double en = SimHits->energy();
+
+ double r = dR(eta_MC, phi_MC, etaS, phiS);
+
+ if ( r < partR ){ // just energy in the small cone
+ enSimHits += en;
+ if(sub == static_cast<int>(HcalBarrel)) enSimHitsHB += en;
There could be some simplification - it would have to be (sub!=subdet_ and subdet_==5) continue, and there would still have to be a check for HF to fill the separate long and short energy deposits. I guess this could also be done when filling the RecHit variables like eHcalConeHB.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
cmsbuild
added
comparison-pending
dqm-pending
pending-signatures
tests-pending
and removed
dqm-approved
fully-signed
tests-approved
labels
|
Pull request #16734 was updated. @cmsbuild, @dmitrijus, @vanbesien, @davidlange6 can you please check and sign again. |
|
@cmsbuild please test |
|
The tests are being triggered in jenkins. |
|
Comparison job queued. |
|
Comparison is ready The workflows 1003.0, 1001.0, 1000.0, 140.53, 136.731, 4.22 have different files in step1_dasquery.log than the ones found in the baseline. You may want to check and retrigger the tests if necessary. You can check it in the "files" directory in the results of the comparisons |
|
+1 |
|
This pull request is fully signed and it will be integrated in one of the next CMSSW_9_0_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @smuzaffar |
cmsbuild
added
dqm-approved
fully-signed
and removed
dqm-pending
pending-signatures
labels
|
+1 |
Restores SimHit vs. RecHit comparisons removed from Validation/HcalRecHits/HcalRecHitsValidation in PR #PR. It adds support for testNumber in SimHit collection which is currently being used in 2023 workflows.