Skip to content
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

[RFC] Add RefToBase member `holder_' in Reflex dicts. #59

Merged
merged 2 commits into from
Aug 12, 2013

Conversation

davidlt
Copy link
Contributor

@davidlt davidlt commented Jul 8, 2013

RefToBase is a templated class with a private memeber holder_ of reftobase::BaseHolder<value_type>* type. The following type is not part of any dictionary, thus holder_ is not serialized.

There are two options:

  1. Make holder_ transient, which would match the current behavior (not saving it), I assume.
  2. Adding a dictionary for reftobase::BaseHolder<value_type>* to make holder_ serializable by ROOT.

Example error generated by ROOT:

Error in <TStreamerInfo::Build:>: edm::RefToBase<reco::Electron>:
edm::reftobase::BaseHolder<reco::Electron>* has no streamer or
dictionary, data member holder_ will not be saved

The following should resolve errors for 9 dictionaries:

  • edm::RefToBase<reco::WMuNuCandidatePtr>
  • edm::RefToBase<reco::WMuNuCandidate>
  • edm::RefToBase<reco::PhotonCore>
  • edm::RefToBase<reco::Photon>
  • edm::RefToBase<reco::Muon>
  • edm::RefToBase<reco::GsfElectronCore>
  • edm::RefToBase<reco::GsfElectron>
  • edm::RefToBase<reco::Electron>
  • edm::RefToBase<PSimHit>

You can test using edmClassStorageSize, e.g.:

edmClassStorageSize 'edm::RefToBase<reco::GsfElectronCore>'

Signed-off-by: David Abdurachmanov David.Abdurachmanov@cern.ch

RefToBase is a templated class with a private memeber holder_ of
reftobase::BaseHolder<value_type>* type. The following type is
not part of any dictionary, thus holder_ is not saved.

Example error generated by ROOT:

Error in <TStreamerInfo::Build:>: edm::RefToBase<reco::Electron>:
edm::reftobase::BaseHolder<reco::Electron>* has no streamer or
dictionary, data member holder_ will not be saved

Signed-off-by: David Abdurachmanov <David.Abdurachmanov@cern.ch>
@ktf
Copy link
Contributor

ktf commented Jul 8, 2013

@Dr15Jones, @wmtan can you look into this and confirm it's ok?

@Dr15Jones
Copy link
Contributor

holder_ must be stored since it holds all the information we need to retrieve the data. BaseHolder is an abstract base class but its concrete implementations ultimately hold an edm::Ref which must be stored. So it looks like we need to generate the missing dictionaries.

@davidlt
Copy link
Contributor Author

davidlt commented Jul 8, 2013

@Dr15Jones, thanks. I will update the branch.

@ktf
Copy link
Contributor

ktf commented Jul 13, 2013

@davidlt any news about this one?

@cmsbuild
Copy link
Contributor

The following categories have been rejected by eulisse (a.k.a. @ktf on GitHub): Analysis

@cms-git-visualization, @cms-git-analysis, @cms-git-core

@cmsbuild
Copy link
Contributor

The following categories have been signed by mikeh (a.k.a. @mdhildreth on GitHub): Full Simulation

@cms-git-simulation, @cms-git-fastsim, @cms-git-operations, @cms-git-geometry

1 similar comment
@cmsbuild
Copy link
Contributor

The following categories have been signed by mikeh (a.k.a. @mdhildreth on GitHub): Full Simulation

@cms-git-simulation, @cms-git-fastsim, @cms-git-operations, @cms-git-geometry

@cmsbuild
Copy link
Contributor

The following categories have been rejected by mikeh (a.k.a. @mdhildreth on GitHub): Full Simulation

@cms-git-simulation, @cms-git-fastsim, @cms-git-operations, @cms-git-geometry

The following is needed for edm::RefToBase<*>, to store holder_
data member.

From Dr15Jones:
  holder_ must be stored since it holds all the information we need
  to retrieve the data. BaseHolder is an abstract base class but its
  concrete implementations ultimately hold an edm::Ref which must be
  stored

Signed-off-by: David Abdurachmanov <David.Abdurachmanov@cern.ch>
@davidlt
Copy link
Contributor Author

davidlt commented Jul 19, 2013

@Dr15Jones , the type (edm::reftobase::BaseHolder<value_type>) itself doesn't seem to have any data members, but I updated pull request to add the missing types to dictionary.

@davidlt davidlt closed this Jul 19, 2013
@davidlt davidlt reopened this Jul 19, 2013
@ktf
Copy link
Contributor

ktf commented Jul 23, 2013

@Dr15Jones can you look into this?

@cmsbuild
Copy link
Contributor

The following categories have been rejected by @slava77: Reconstruction

@cms-git-reconstruction

@slava77
Copy link
Contributor

slava77 commented Jul 23, 2013

oops, how do I undo the rejection?
there is no confirm button and the text jumps up and down when I click a
button nearby.

On 7/23/13 11:14 AM, cmsbuild wrote:

The following categories have been rejected by @slava77
https://github.com/slava77: Reconstruction

@cms-git-reconstruction


Reply to this email directly or view it on GitHub
#59 (comment).


Vyacheslav (Slava) Krutelyov
TAMU: Physics Dept Texas A&M MS4242, College Station, TX 77843-4242
CERN: 42-R-027
AIM/Skype: siava16 googleTalk: slava77@gmail.com
(630) 291-5128 Cell (US) +41 76 275 7116 Cell (CERN)


@ktf
Copy link
Contributor

ktf commented Jul 23, 2013

Click on sign and it will move it from the rejected state to the approved state.

@slava77
Copy link
Contributor

slava77 commented Jul 23, 2013

On 7/23/13 11:29 AM, Giulio Eulisse wrote:

Click on sign and it will move it from the rejected state to the
approved state.

will it change the state to "signed"? I don't want to sign it until I
test it.
There are other rejection signatures in this request already,
I don't see a point of testing it until those are cleared.


Reply to this email directly or view it on GitHub
#59 (comment).


Vyacheslav (Slava) Krutelyov
TAMU: Physics Dept Texas A&M MS4242, College Station, TX 77843-4242
CERN: 42-R-027
AIM/Skype: siava16 googleTalk: slava77@gmail.com
(630) 291-5128 Cell (US) +41 76 275 7116 Cell (CERN)


@Dr15Jones
Copy link
Contributor

This change looks fine to me. I double checked that classes which inherited from HolderBase have dictionaries declared and they do

@slava77
Copy link
Contributor

slava77 commented Jul 23, 2013

@Dr15Jones it's nice to hear this is ok for the CORE.
@ktf Why was it rejected for Analysis?
@mdhildreth Why was it rejected for FullSim?

@ktf
Copy link
Contributor

ktf commented Jul 23, 2013

I think I rejected by mistake while trying out something for Volker. @vadler, can you check and approve?

@ktf
Copy link
Contributor

ktf commented Aug 2, 2013

@vadler can you please look and approve?

@ktf
Copy link
Contributor

ktf commented Aug 5, 2013

@vadler ???

@ghost ghost assigned vadler Aug 5, 2013
@vadler
Copy link

vadler commented Aug 5, 2013

I'm catching up with GIT stuff after vacation. This one will be done tomorrow morning.
BTW: I can see some more rejects in the TC.

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 6, 2013

The following categories have been signed by @vadler: Analysis

@cms-git-analysis

1 similar comment
@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 6, 2013

The following categories have been signed by @vadler: Analysis

@cms-git-analysis

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 9, 2013

The following categories have been signed by mikeh (a.k.a. @mdhildreth on GitHub): Full Simulation

@cms-git-simulation, @cms-git-fastsim, @cms-git-operations, @cms-git-geometry

@ktf
Copy link
Contributor

ktf commented Aug 9, 2013

@nclopezo can you test it, please?

@nclopezo
Copy link
Contributor

nclopezo commented Aug 9, 2013

Hi,

I tested these changes in CMSSW_7_0_X_2013-08-09-0200. All tests passed.

You can see the logs in:

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

ktf added a commit that referenced this pull request Aug 12, 2013
[RFC] Add RefToBase member `holder_'  in Reflex dicts.
@ktf ktf merged commit 8377aee into cms-sw:CMSSW_7_0_X Aug 12, 2013
@ktf
Copy link
Contributor

ktf commented Aug 12, 2013

As discussed in ORP, and given we are at the beginning pre3, I simply approve it.

mileva pushed a commit to mileva/cmssw that referenced this pull request Feb 15, 2014
nclopezo added a commit to nclopezo/cmssw that referenced this pull request May 5, 2014
It filters the tags using the release queue
gkasieczka pushed a commit to gkasieczka/cmssw that referenced this pull request May 19, 2015
I have no time to check it now but as I could be offline in the next days I merge it
gdimperi pushed a commit to gdimperi/cmssw that referenced this pull request Jun 21, 2015
Welcome to the `development` branch.
gpetruc pushed a commit to gpetruc/cmssw that referenced this pull request Jul 2, 2015
For MC, turn on again mc matching for photons
akalinow pushed a commit to akalinow/cmssw that referenced this pull request Oct 19, 2015
…microGMT_paramsESProducer

MicroGMT parameters and LUTs in CondDB
arizzi pushed a commit to arizzi/cmssw that referenced this pull request Dec 17, 2015
Thanks, Andrea. I will now have a look at your other PR
yetkinyilmaz pushed a commit to yetkinyilmaz/cmssw that referenced this pull request Feb 18, 2016
jbsauvan added a commit to jbsauvan/cmssw that referenced this pull request Oct 14, 2016
mariadalfonso pushed a commit to mariadalfonso/cmssw that referenced this pull request May 6, 2017
cmsbuild pushed a commit that referenced this pull request Sep 24, 2017
mseidel42 pushed a commit to mseidel42/cmssw that referenced this pull request Oct 31, 2017
Recompute miniIso on the fly for 80X miniAODs
fwyzard added a commit to vkhristenko/cmssw that referenced this pull request May 29, 2018
tahuang1991 pushed a commit to tahuang1991/cmssw that referenced this pull request Sep 13, 2018
felicepantaleo pushed a commit to felicepantaleo/cmssw that referenced this pull request Mar 28, 2019
update code to cope with old and new geometry and old and new releases
gpetruc pushed a commit to gpetruc/cmssw that referenced this pull request Oct 29, 2019
tomalin pushed a commit to tomalin/cmssw that referenced this pull request Dec 21, 2020
* Fixing the disp tracking bug

Changed a couple of asserts into if statements

* fixing warning

Fixed the following warning:

/cms/threejet-2/abhijith/hware_temp/approx_cal/CMSSW_11_2_0_pre6/src/L1Trigger/TrackFindingTracklet/src/MatchCalculator.cc:482:7: warning: 'imatch' may be used uninitialized in this function [-Wmaybe-uninitialized]

* add use approx switch to Tracklet Calculator Displaced
tomalin pushed a commit to tomalin/cmssw that referenced this pull request Jan 5, 2021
* Fixing the disp tracking bug

Changed a couple of asserts into if statements

* fixing warning

Fixed the following warning:

/cms/threejet-2/abhijith/hware_temp/approx_cal/CMSSW_11_2_0_pre6/src/L1Trigger/TrackFindingTracklet/src/MatchCalculator.cc:482:7: warning: 'imatch' may be used uninitialized in this function [-Wmaybe-uninitialized]

* add use approx switch to Tracklet Calculator Displaced
tomalin pushed a commit to tomalin/cmssw that referenced this pull request Mar 3, 2021
* Fixing the disp tracking bug

Changed a couple of asserts into if statements

* fixing warning

Fixed the following warning:

/cms/threejet-2/abhijith/hware_temp/approx_cal/CMSSW_11_2_0_pre6/src/L1Trigger/TrackFindingTracklet/src/MatchCalculator.cc:482:7: warning: 'imatch' may be used uninitialized in this function [-Wmaybe-uninitialized]

* add use approx switch to Tracklet Calculator Displaced
cerminar pushed a commit to cerminar/cmssw that referenced this pull request Apr 30, 2021
Update E/gamma emulators and regionizer to support realistic layer1 endcap setup
franzoni pushed a commit to franzoni/cmssw that referenced this pull request May 3, 2021
* Updating the closeby gun with randomShoot option
drankincms pushed a commit to drankincms/cmssw that referenced this pull request Mar 9, 2022
Some first support for lepton trigger plots in the validation tools
yulunmiao pushed a commit to yulunmiao/cmssw that referenced this pull request Oct 4, 2023
…O-13_2_0_pre3_dqmfeat

Use Si Cell information in HGCalDigisClient
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants