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

Extend the hit pattern class to include MTD hits #24419

Merged
merged 7 commits into from Oct 10, 2018

Conversation

lgray
Copy link
Contributor

@lgray lgray commented Aug 30, 2018

Extends the reco::HitPattern class to understand timing detector hits.
HitPattern is now 12 bits instead of 11, appropriate iorule added to maintain compatibility.

As discussed with @VinInn and company.

@fabiocos @bendavid @casarsa

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@lgray
Copy link
Contributor Author

lgray commented Aug 30, 2018

As a note: I'll rebase this ontop of #24285 when it gets merged, since the public -> private changes for MTD/BTL/ETL DetIds are there.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

DataFormats/ForwardDetId
DataFormats/TrackReco

@perrotta, @civanch, @kpedro88, @cmsbuild, @mdhildreth, @slava77 can you please review it and eventually sign? Thanks.
@makortel, @gpetruc, @rovere, @VinInn this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

@lgray
Copy link
Contributor Author

lgray commented Aug 30, 2018

@cmsbuild please test workflow 22434.0

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 30, 2018

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/30154/console Started: 2018/08/30 18:21
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/30161/console Started: 2018/08/30 21:35

@cmsbuild
Copy link
Contributor

There was an issue with git-cms-merge-topic you can see the log here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-24419/30154/git-merge-result

@lgray
Copy link
Contributor Author

lgray commented Aug 30, 2018

Someone please kick this when all the plumbing is working again.

@slava77
Copy link
Contributor

slava77 commented Aug 30, 2018

the console has an error

there are staged but not committed changes on your working tree, please commit or stash them.

@smuzaffar did something go bad with some recent git updates?

@smuzaffar
Copy link
Contributor

no idea but I am looking in to it.

@smuzaffar
Copy link
Contributor

this could be related to the changes here cms-sw/cms-git-tools#99
For now I disabled the sourcing of /cvmfs/cms-ib.cern.ch/nweek-02539/cmsset_default.sh and restarted the tests.

@cmsbuild
Copy link
Contributor

-1

Tested at: b185fb3

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
a5b1e46
fc59a02
2d95ea1
a1dc06b
3d33c11
c6eda53
8fbe2ee
You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-24419/30161/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-24419/30161/git-merge-result

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-24419/30161/summary.html

I found follow errors while testing this PR

Failed tests: Build HeaderConsistency

  • Build:

I found an error when building:

/cvmfs/cms-ib.cern.ch/week1/slc6_amd64_gcc700/cms/cmssw-patch/CMSSW_10_3_X_2018-08-30-1100/src/FWCore/MessageLogger/interface/ErrorObj.icc: In instantiation of 'edm::ErrorObj& edm::ErrorObj::opltlt(const T&) [with T = DetId]':
/cvmfs/cms-ib.cern.ch/week1/slc6_amd64_gcc700/cms/cmssw-patch/CMSSW_10_3_X_2018-08-30-1100/src/FWCore/MessageLogger/interface/ErrorObj.icc:44:20:   required from 'edm::ErrorObj& edm::operator<<(edm::ErrorObj&, const T&) [with T = DetId]'
/cvmfs/cms-ib.cern.ch/week1/slc6_amd64_gcc700/cms/cmssw-patch/CMSSW_10_3_X_2018-08-30-1100/src/FWCore/MessageLogger/interface/MessageSender.h:47:32:   required from 'edm::MessageSender& edm::MessageSender::operator<<(const T&) [with T = DetId]'
/cvmfs/cms-ib.cern.ch/week1/slc6_amd64_gcc700/cms/cmssw-patch/CMSSW_10_3_X_2018-08-30-1100/src/FWCore/MessageLogger/interface/MessageLogger.h:184:51:   required from 'edm::LogError& edm::LogError::operator<<(const T&) [with T = DetId]'
/build/cmsbld/jenkins/workspace/ib-any-integration/CMSSW_10_3_X_2018-08-30-1100/src/RecoTracker/TkSeedGenerator/src/SeedFromProtoTrack.cc:75:133:   required from here
/cvmfs/cms-ib.cern.ch/week1/slc6_amd64_gcc700/cms/cmssw-patch/CMSSW_10_3_X_2018-08-30-1100/src/FWCore/MessageLogger/interface/ErrorObj.icc:28:8: error: ambiguous overload for 'operator<<' (operand types are 'std::ostringstream {aka std::__cxx11::basic_ostringstream}' and 'const DetId')
   myOs << t;
   ~~~~~^~~~
In file included from /cvmfs/cms-ib.cern.ch/nweek-02539/slc6_amd64_gcc700/external/gcc/7.0.0-omkpbe2/include/c++/7.3.1/istream:39:0,
                 from /cvmfs/cms-ib.cern.ch/nweek-02539/slc6_amd64_gcc700/external/gcc/7.0.0-omkpbe2/include/c++/7.3.1/sstream:38,
                 from /cvmfs/cms-ib.cern.ch/week1/slc6_amd64_gcc700/cms/cmssw-patch/CMSSW_10_3_X_2018-08-30-1100/src/FWCore/Utilities/interface/Exception.h:36,


The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
a5b1e46
fc59a02
2d95ea1
a1dc06b
3d33c11
c6eda53
8fbe2ee
You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-24419/30161/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-24419/30161/git-merge-result

@cmsbuild
Copy link
Contributor

Comparison not run due to Build errors (RelVals and Igprof tests were also skipped)

@lgray
Copy link
Contributor Author

lgray commented Sep 28, 2018

@fwyzard @Martin-Grunewald could you please sign? Thanks!

@Martin-Grunewald
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 1, 2018

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)

@fabiocos
Copy link
Contributor

fabiocos commented Oct 2, 2018

please test workflow 22434.0

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 2, 2018

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/30835/console Started: 2018/10/02 10:05

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 2, 2018

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 2, 2018

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 2, 2018

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 190 differences found in the comparisons
  • DQMHistoTests: Total files compared: 32
  • DQMHistoTests: Total histograms compared: 3162800
  • DQMHistoTests: Total failures: 2
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3162601
  • DQMHistoTests: Total skipped: 197
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 31 files compared)
  • Checked 134 log files, 14 edm output root files, 32 DQM output files

@fabiocos
Copy link
Contributor

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants