-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Phase2-hgx119 HGCDigi can work with generic DetId #23690
Conversation
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-23690/5349 |
A new Pull Request was created by @bsunanda for master. It involves the following packages: DataFormats/HGCDigi @cmsbuild, @civanch, @kpedro88, @mdhildreth can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
@cmsbuild Please test |
The tests are being triggered in jenkins. |
@fabiocos This may be the last change to DataFormats |
@bsunanda I do think this makes more sense in general. But a few questions/comments:
However, if we keep the old dataframe types defined, we can keep the first template parameter in |
@kpedro88 I agree with you. I shall restore the old definitions in classes.h and the xml files. Do I need to change the typedef for DigiCollections? Are Digi's stored as persistent objects? |
Yes, digis are stored persistently, so it's better not to change the typedefs. |
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-23690/5351 |
@kpedro88 Could you please take care of them? I am concentrating on geometry issue now |
@cmsbuild Please test |
The tests are being triggered in jenkins. |
@bsunanda okay, I will do it once this is merged. |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
+1 |
+1 |
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2) |
@@ -18,22 +24,6 @@ namespace DataFormats_HGCDigi { | |||
std::vector<HGCDataFrame<HcalDetId,HGCSample> > vHGCalBHDataFrames; | |||
edm::SortedCollection< HGCDataFrame<HcalDetId,HGCSample> > scHGCalBHDataFrames; | |||
edm::Wrapper< edm::SortedCollection< HGCDataFrame<HcalDetId,HGCSample> > > prodHGCalBHDataFrames; | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fabiocos yes, that's why we kept them here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kpedro88 sorry, I obviously mean the removed ones: anHGCEEDataFrame, anHGCHEDataFrame...
I tried to explain that the removed ones were not used at least for 2 years once the hexagonal design is chosen and became operational. The codes for square cells do not exist in other parts of CMSSW. So there is no scope to use them anywhere anymore
________________________________
From: Fabio Cossutti [[email protected]]
Sent: 06 July 2018 09:33
To: cms-sw/cmssw
Cc: Sunanda Banerjee; Mention
Subject: Re: [cms-sw/cmssw] Phase2-hgx119 HGCDigi can work with generic DetId (#23690)
@fabiocos commented on this pull request.
________________________________
In DataFormats/HGCDigi/src/classes.h<#23690 (comment)>:
@@ -18,22 +24,6 @@ namespace DataFormats_HGCDigi {
std::vector<HGCDataFrame<HcalDetId,HGCSample> > vHGCalBHDataFrames;
edm::SortedCollection< HGCDataFrame<HcalDetId,HGCSample> > scHGCalBHDataFrames;
edm::Wrapper< edm::SortedCollection< HGCDataFrame<HcalDetId,HGCSample> > > prodHGCalBHDataFrames;
-
@kpedro88<https://github.com/kpedro88> sorry, I obviously mean the removed ones: anHGCEEDataFrame, anHGCHEDataFrame...
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#23690 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AEzMurFXoucFJSLJB8ABbMRvaFxvjd3Bks5uDxLIgaJpZM4U4iqP>.
|
+1 |
DetId class changed for HGC from HGCal to HGCSilicon and from Hcal to HGCScintillator. Accordingly Digi collection now depends on generic DetId