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

Jump to bottom

Implement edm::View support for BXVector #13562

Merged
merged 1 commit into from Mar 3, 2016
Merged

Implement edm::View support for BXVector #13562

merged 1 commit into from Mar 3, 2016

Conversation

Martin-Grunewald
Copy link
Contributor

Implement edm::View support for BXVector

@cmsbuild cmsbuild added this to the Next CMSSW_8_0_X milestone Mar 2, 2016
@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 2, 2016

A new Pull Request was created by @Martin-Grunewald (Martin Grunewald) for CMSSW_8_0_X.

It involves the following packages:

DataFormats/L1Trigger

@cmsbuild, @mulhearn, @davidlange6 can you please review it and eventually sign? Thanks.
@slava77, @Degano, @smuzaffar you are the release manager for this.

cms-bot commands are list here #13028

@Martin-Grunewald
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 2, 2016

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/11675/console

@Martin-Grunewald
Copy link
Contributor Author

@Dr15Jones
Could you please have a look?
Thanks!

@Martin-Grunewald Martin-Grunewald mentioned this pull request Mar 2, 2016
Open
@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 3, 2016

+1
Tested at: 106cf28
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-13562/11675/summary.html

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
cad854b
8e8ed3d
f89595f
0e27df5
fdddf7a
dc44cd8
e4b1500
a5c91fe
440e538
0485b63
c759cab
5c54649
98e00d0
126eaa8
You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-13562/11675/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-13562/11675/git-merge-result

@mulhearn
Copy link
Contributor

mulhearn commented Mar 3, 2016

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 3, 2016

This pull request is fully signed and it will be integrated in one of the next CMSSW_8_0_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @Degano, @smuzaffar

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 3, 2016

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-13562/11675/summary.html

@davidlange6
Copy link
Contributor

+1

cmsbuild added a commit that referenced this pull request Mar 3, 2016
@cmsbuild
Merge pull request #13562 from Martin-Grunewald/BXVectorView
e63bd64 
Implement edm::View support for BXVector
@cmsbuild cmsbuild merged commit e63bd64 into cms-sw:CMSSW_8_0_X Mar 3, 2016
@Dr15Jones
Copy link
Contributor

If you also wanted to support edm::Ptr access (an edm::View does let one get an edm::Ptr for each item in the collection) then you'd follow the example in edm::OwnVector

http://cmslxr.fnal.gov/source/DataFormats/Common/interface/OwnVector.h#0536

@Martin-Grunewald
Copy link
Contributor Author

@Dr15Jones
Well, if I add that part, I get more than 2000 lines of compiler errors which I do not understand.
The files are at /afs/cern.ch/user/g/gruen/public
There must be something else missing in the implementation of T for BXVector (T)
Some cut outs:

                 from DataFormatsL1Trigger/a/DataFormatsL1Trigger_xr.cc:41:
/cvmfs/cms-ib.cern.ch/week0/slc6_amd64_gcc493/cms/cmssw/CMSSW_8_0_X_2016-03-03-1300/src/DataFormats/Common/interface/WrapperView.icc: In instantiation of 'static void edm::helpers::PtrSetter<T>::set(const T&, const std::type_info&, long unsigned int, const void*&) [with T = BXVector<l1t::L1DataEmulResult>]':
/cvmfs/cms-ib.cern.ch/week0/slc6_amd64_gcc493/cms/cmssw/CMSSW_8_0_X_2016-03-03-1300/src/DataFormats/Common/interface/WrapperView.icc:110:58:   required from 'void edm::DoSetPtr<T>::operator()(const T&, const std::type_info&, long unsigned int, const void*&) const [with T = BXVector<l1t::L1DataEmulResult>]'
/cvmfs/cms-ib.cern.ch/week0/slc6_amd64_gcc493/cms/cmssw/CMSSW_8_0_X_2016-03-03-1300/src/DataFormats/Common/interface/WrapperView.icc:62:50:   required from 'void edm::Wrapper<T>::do_setPtr(const std::type_info&, long unsigned int, const void*&) const [with T = BXVector<l1t::L1DataEmulResult>]'
DataFormatsL1Trigger/a/DataFormatsL1Trigger_xr.cc:7844:1:   required from here
/cvmfs/cms-ib.cern.ch/week0/slc6_amd64_gcc493/cms/cmssw/CMSSW_8_0_X_2016-03-03-1300/src/DataFormats/Common/interface/WrapperView.icc:92:44: error: no matching function for call to 'setPtr(const BXVector<l1t::L1DataEmulResult>&, const std::type_info&, long unsigned int&, const void*&)'
           setPtr(obj, iToType, iIndex, oPtr);

or

/cvmfs/cms-ib.cern.ch/week0/slc6_amd64_gcc493/cms/cmssw/CMSSW_8_0_X_2016-03-03-1300/src/DataFormats/Common/interface/WrapperView.icc:92:44: note: candidates are:
In file included from /cvmfs/cms-ib.cern.ch/week0/slc6_amd64_gcc493/cms/cmssw/CMSSW_8_0_X_2016-03-03-1300/src/DataFormats/Common/interface/WrapperView.icc:121:0,
                 from /cvmfs/cms-ib.cern.ch/week0/slc6_amd64_gcc493/cms/cmssw/CMSSW_8_0_X_2016-03-03-1300/src/DataFormats/Common/interface/Wrapper.h:149,
                 from /cvmfs/cms-ib.cern.ch/week0/slc6_amd64_gcc493/cms/cmssw/CMSSW_8_0_X_2016-03-03-1300/src/DataFormats/Common/interface/RefCoreGet.h:12,
                 from /cvmfs/cms-ib.cern.ch/week0/slc6_amd64_gcc493/cms/cmssw/CMSSW_8_0_X_2016-03-03-1300/src/DataFormats/Common/interface/RefProd.h:169,
                 from /cvmfs/cms-ib.cern.ch/week0/slc6_amd64_gcc493/cms/cmssw/CMSSW_8_0_X_2016-03-03-1300/src/DataFormats/Common/interface/Ref.h:455,
                 from /cvmfs/cms-ib.cern.ch/week0/slc6_amd64_gcc493/cms/cmssw/CMSSW_8_0_X_2016-03-03-1300/src/DataFormats/Common/interface/OwnVector.h:7,
                 from /cvmfs/cms-ib.cern.ch/week0/slc6_amd64_gcc493/cms/cmssw/CMSSW_8_0_X_2016-03-03-1300/src/DataFormats/Candidate/interface/CandidateFwd.h:3,
                 from /cvmfs/cms-ib.cern.ch/week0/slc6_amd64_gcc493/cms/cmssw/CMSSW_8_0_X_2016-03-03-1300/src/DataFormats/Candidate/interface/const_iterator.h:9,
                 from /cvmfs/cms-ib.cern.ch/week0/slc6_amd64_gcc493/cms/cmssw/CMSSW_8_0_X_2016-03-03-1300/src/DataFormats/Candidate/interface/Candidate.h:12,
                 from /cvmfs/cms-ib.cern.ch/week0/slc6_amd64_gcc493/cms/cmssw/CMSSW_8_0_X_2016-03-03-1300/src/DataFormats/Candidate/interface/LeafCandidate.h:11,
                 from /scratch/CMS3/l1t/CMSSW_8_0_X_2016-03-03-1300/src/DataFormats/L1Trigger/interface/L1EmParticle.h:20,
                 from /scratch/CMS3/l1t/CMSSW_8_0_X_2016-03-03-1300/src/DataFormats/L1Trigger/src/classes.h:1,
                 from DataFormatsL1Trigger/a/DataFormatsL1Trigger_xr.cc:41:
/cvmfs/cms-ib.cern.ch/week0/slc6_amd64_gcc493/cms/cmssw/CMSSW_8_0_X_2016-03-03-1300/src/DataFormats/Common/interface/setPtr.h:72:3: note: template<class T, class A> void edm::setPtr(const std::vector<_Tp, _Alloc>&, const std::type_info&, long unsigned int, const void*&)
   setPtr(std::vector<T, A> const& obj,
   ^

or

/cvmfs/cms-ib.cern.ch/week0/slc6_amd64_gcc493/cms/cmssw/CMSSW_8_0_X_2016-03-03-1300/src/DataFormats/Common/interface/setPtr.h:72:3: note:   template argument deduction/substitution failed:
In file included from /cvmfs/cms-ib.cern.ch/week0/slc6_amd64_gcc493/cms/cmssw/CMSSW_8_0_X_2016-03-03-1300/src/DataFormats/Common/interface/Wrapper.h:149:0,
                 from /cvmfs/cms-ib.cern.ch/week0/slc6_amd64_gcc493/cms/cmssw/CMSSW_8_0_X_2016-03-03-1300/src/DataFormats/Common/interface/RefCoreGet.h:12,
                 from /cvmfs/cms-ib.cern.ch/week0/slc6_amd64_gcc493/cms/cmssw/CMSSW_8_0_X_2016-03-03-1300/src/DataFormats/Common/interface/RefProd.h:169,
                 from /cvmfs/cms-ib.cern.ch/week0/slc6_amd64_gcc493/cms/cmssw/CMSSW_8_0_X_2016-03-03-1300/src/DataFormats/Common/interface/Ref.h:455,
                 from /cvmfs/cms-ib.cern.ch/week0/slc6_amd64_gcc493/cms/cmssw/CMSSW_8_0_X_2016-03-03-1300/src/DataFormats/Common/interface/OwnVector.h:7,
                 from /cvmfs/cms-ib.cern.ch/week0/slc6_amd64_gcc493/cms/cmssw/CMSSW_8_0_X_2016-03-03-1300/src/DataFormats/Candidate/interface/CandidateFwd.h:3,
                 from /cvmfs/cms-ib.cern.ch/week0/slc6_amd64_gcc493/cms/cmssw/CMSSW_8_0_X_2016-03-03-1300/src/DataFormats/Candidate/interface/const_iterator.h:9,
                 from /cvmfs/cms-ib.cern.ch/week0/slc6_amd64_gcc493/cms/cmssw/CMSSW_8_0_X_2016-03-03-1300/src/DataFormats/Candidate/interface/Candidate.h:12,
                 from /cvmfs/cms-ib.cern.ch/week0/slc6_amd64_gcc493/cms/cmssw/CMSSW_8_0_X_2016-03-03-1300/src/DataFormats/Candidate/interface/LeafCandidate.h:11,
                 from /scratch/CMS3/l1t/CMSSW_8_0_X_2016-03-03-1300/src/DataFormats/L1Trigger/interface/L1EmParticle.h:20,
                 from /scratch/CMS3/l1t/CMSSW_8_0_X_2016-03-03-1300/src/DataFormats/L1Trigger/src/classes.h:1,
                 from DataFormatsL1Trigger/a/DataFormatsL1Trigger_xr.cc:41:
/cvmfs/cms-ib.cern.ch/week0/slc6_amd64_gcc493/cms/cmssw/CMSSW_8_0_X_2016-03-03-1300/src/DataFormats/Common/interface/WrapperView.icc:92:44: note:   'const BXVector<l1t::L1DataEmulResult>' is not derived from 'const std::vector<_Tp, _Alloc>'
           setPtr(obj, iToType, iIndex, oPtr);

@Dr15Jones
Copy link
Contributor

@Martin-Grunewald could you make the log containing all the errors available somewhere?

@Dr15Jones
Copy link
Contributor

@Martin-Grunewald I see that you did make them available. Guess the coffee hasn't kicked in yet this morning :).

@Dr15Jones
Copy link
Contributor

This seems to have done the trick

  template <class T>
  inline
  void
  setPtr(BXVector<T> const& obj,
         std::type_info const& toType,
         unsigned long index,
         void const*& ptr) {
    obj.setPtr(toType, index, ptr);
  }
  template <class T>
  inline
  void
  fillPtrVector(BXVector<T> const& obj,
                std::type_info const& toType,
                std::vector<unsigned long> const& indices,
                std::vector<void const*>& ptrs) {
    obj.fillPtrVector(toType, indices, ptrs);
  }
namespace edm {
  template <class T>
  struct has_setPtr<BXVector<T> > {
    static bool const value = true;
  };
}

I.e. setPtr and fillPtrVector both need to be in the global namespace. The reason is how functions are found in C++. If you call foo( bar ) where bar is in a namespace, C++ will first look for foo in the same namespace as bar and if that fails it will look for foo in the global namespace. In the example I had you look at, OwnVector resides in the edm namespace, hence why the two stand alone functions were also in that namespace. But your class resides in the global namespace so the functions need to do that as well.

@Martin-Grunewald
Copy link
Contributor Author

@Dr15Jones
Great, thanks a lot! Done in #13602

@Martin-Grunewald Martin-Grunewald deleted the BXVectorView branch March 22, 2016 07:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
comparison-available fully-signed l1-approved orp-approved tests-approved
Projects
None yet
Milestone
CMSSW_8_0_X
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@Martin-Grunewald @cmsbuild @mulhearn @davidlange6 @Dr15Jones