Ecal regression refactor #5423
Ecal regression refactor #5423
Conversation
… variables rather than code copy and pastes as is currently
…EcalRegressionData is identical to the existing one
…ression code, this changed what is stored but hopefully for the better
|
A new Pull Request was created by @Sam-Harper for CMSSW_7_2_X. Ecal regression refactor It involves the following packages: PhysicsTools/PatAlgos @nclopezo, @monttj, @cmsbuild, @StoyanStoynev, @slava77, @vadler can you please review it and eventually sign? Thanks. |
| for( auto clus = superClus.clustersBegin()+1;clus != superClus.clustersEnd(); ++clus ) { | ||
| const float dEta = (*clus)->eta() - superClus.seed()->eta(); | ||
| const float dPhi = reco::deltaPhi((*clus)->phi(),superClus.seed()->phi()); | ||
| const float dR = std::hypot(dEta,dPhi); |
There was a problem hiding this comment.
You should avoid the square root (use deltaR2()).
There was a problem hiding this comment.
yeah I saw that when I was porting the code, fine I changed it to be the deltaR2 internally and updated methods as a result.
|
-1 Tested at: a074f0e ---> test runtestPhysicsToolsPatAlgos had ERRORS you can see the results of the tests here: |
|
+1 |
|
This pull request is fully signed and it will be integrated in one of the next CMSSW_7_2_X IBs unless changes (tests are also fine). @nclopezo can you please take care of it? |
This code refactors the ECAL regression inputs so a new single class gets the data rather than the same (buggy) code being copy & pasted over 3 files. It also fixes a divide by zero bug. It changes the behaviour of the pat::Photons but for the better in my opinion.