Reco Updates -- Add calibrated energy to CaloCluster

[lgray]

This is to reduce the number of additional intermediate collections needed for the particle flow ECAL clusters and super clusters.
It doesn't interfere with any of the current cluster producers.
Based on CMSSW_7_0_0_pre3.

[cmsbuild]

A new Pull Request was created by @lgray (Lindsey Gray) for CMSSW_7_0_X.

add calibrated energy to CaloCluster

It involves the following packages:

DataFormats/CaloRecHit

@thspeer, @slava77 can you please review it and eventually sign? Thanks.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.

[slava77]

Hi Lindsey.
Could you please be more specific on the savings.
Which collections will go, which will stay.

Thanks

[lgray]

Hi Slava,

This is basically an addition for PF so we don't have to cast back to
PFCluster all the time.
As it stands it would remove a duplication of list of PF-ECAL clusters.
Likewise, one could remove one processing step (and a collection) for each
ECAL cluster variant, were they to be changed.

Best,
-Lindsey

On Mon, Sep 2, 2013 at 11:45 AM, slava77 notifications@github.com wrote:

Hi Lindsey.
Could you please be more specific on the savings.
Which collections will go, which will stay.

Thanks

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/686#issuecomment-23650395
.

[nclopezo]

Hi,

I ran the tests for this pull request on top of CMSSW_7_0_X_2013-09-02-0200 all tests passed:

https://cmssdt.cern.ch/jenkins/job/Pull-Request-Integration/ARCHITECTURE=slc5_amd64_gcc472/363/consoleFull

you can see the artifacts here:

https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/363/

[slava77]

@lgray @Dr15Jones

I was expecting to still be able to read the previous version of the objects with the new dictionary version.
Chris, is it one of the things where we need to increase the class version for all derived classes?

[Dr15Jones]

That's not the way I understand how ROOT's schema evolution is supposed to work.

[Dr15Jones]

@slava77 Slava, what did you see that made you conclude that you couldn't read the old format?

[slava77]

@Dr15Jones

Right expected, but still, I can read the base class with the change, but I can't read the derived one.
To be specific: in CMSSW_7_0_X_2013-09-10-0200-original make wf 1000.0 (T0 reco) output, repeat the same with CMSSW_7_0_X_2013-09-10-0200 + #686.

e = new TChain("Events")
e->Add("newFile");
eo = new TChain("Events")
eo->Add("oldFile");

e->Draw("recoCaloClusters_multi5x5SuperClusters_multi5x5EndcapBasicClusters_RECO.obj@.size()")
eo->Draw("recoCaloClusters_multi5x5SuperClusters_multi5x5EndcapBasicClusters_RECO.obj@.size()")
both work ok

e->Draw("recoPreshowerClusters_multi5x5SuperClustersWithPreshower_preshowerXClusters_RECO.obj@.size()")
works
eo->Draw("recoPreshowerClusters_multi5x5SuperClustersWithPreshower_preshowerXClusters_RECO.obj@.size()")
breaks miserably
(in batch I gave it a try a bit longer, but it never succeeded even after ~60GB of error messages)

Not that long ago (las Fall, maybe) we had to increase some derived classes version numbers when changing something simple in a base class (was it TrackBase or smth, don't recall). Back then it was blamed on some bug in reflex.

[pcanal]

What is the first error message?

[slava77]

Hi Philippe

eo->Draw("recoPreshowerClusters_multi5x5SuperClustersWithPreshower_preshowerXClusters_RECO.obj@.size()"); >& eoPR686.log

Error in TBufferFile::ReadClassBuffer: class: ROOT::Math::PositionVector3DROOT::Math::Cartesian3D<double,ROOT::Math::DefaultCoordinateSystemTag>, attempting to access a wrong version: 16499, object skipped at offset 259
Error in TBufferFile::CheckByteCount: object of class ROOT::Math::PositionVector3DROOT::Math::Cartesian3D<double,ROOT::Math::DefaultCoordinateSystemTag> read too few bytes: 2 instead of 793307784
Error in TBufferFile::CheckByteCount: Byte count probably corrupted around buffer position 253:
793307784 for a possible maximum of 17149
Error in TBufferFile::ReadVersion: Could not find the StreamerInfo with a checksum of 0x0 for the class "ROOT::Math::PositionVector3DROOT::Math::Cartesian3D<double,ROOT::Math::DefaultCoordinateSystemTag>" in oldFile.

Then it says something pretty similar for all the elements of the data members of the class.

I put the first few seconds of the errors in
/afs/cern.ch/user/s/slava77/public/logs/eoPR686.log

[pcanal]

Does the problem disappear if you also change the derived class version number? (Especially if it does, I would interested in taking a look (for future improvement) at the input root file).

Thanks,
Philippe.

[slava77]

Hi Philippe,

Yes, the problem disappears after I increment the class version for one of the derived classes:

DataFormats/EgammaReco/src/classes_def.xml

• <class name="reco::PreshowerCluster" ClassVersion="10">
• <class name="reco::PreshowerCluster" ClassVersion="11">
• <version ClassVersion="11" checksum="469230288"/>

https://github.com/cms-sw/cmssw/blob/CMSSW_7_0_X/DataFormats/EgammaReco/src/classes_def.xml

Should I go on change this for all other classes (not such a fun exercise)

Slava

[slava77]

d'oh the code snippet does not display well
the commit is here in
slava77@1a556a6

[pcanal]

Should I go on change this for all other classes (not such a fun exercise)
This would 'guarantee' success but is like a superset of what is needed. The one that are needed would be any of the derived class that are used in a split branch.

[slava77]

well, considering there were just two more classes deriving from CaloCluster, I just changed all.

@lgray
Lindsey, could you please pick up my two commits 1a556a6 and 4711698 (or reapply) in your branch, so that we keep to the same pull request.

[lgray]

@slava77 commits finally added after github weirdness

[cmsbuild]

Pull request #686 was updated. @nclopezo, @smuzaffar, @thspeer, @slava77 can you please check and sign again.

[lgray]

@slava77 A minor delay, the DPG conveners want a bit of time to think a bit more about the naming.

[slava77]

@lgray
Hi Lindsey,
No problem.
Let me know when you expect an update or a decision to go with this name or another.

[slava77]

@lgray
Hi Lindsey.
Any news here?

[lgray]

@slava77 Yes, about to send a new commit.

[cmsbuild]

Pull request #686 was updated. @nclopezo, @smuzaffar, @thspeer, @slava77 can you please check and sign again.

[nclopezo]

+1
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/764/summary.html

[slava77]

+1

tested 5d78f7b in CMSSW_7_0_X_2013-10-03-1400
(note to self: this was test area sign253)
no differences as expected
reading both old and new versions of clusters also works

[cmsbuild]

This pull request is fully signed and it will be integrated in one of the next IBs unless changes or unless it breaks tests. @ktf can you please take care of it?