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

CTPPS: strips update for 2017 data-taking #18283

Merged
merged 15 commits into from Apr 27, 2017

Conversation

jan-kaspar
Copy link
Contributor

This PR contains changes in the CT-PPS Si strip SW needed for the 2017 data-taking:

  • Updated list of FED ids: including both horizontal and vertical Roman Pots (RPs).
  • Updated cabling mapping.
  • Updated DQM for strips: plots are booked only for RPs containing strip sensors.

runTheMatrix.py -l limited was executed in CMSSW_9_1_X_2017-04-07-1100 yielding

	16 16 15 13 7 1 1 1 tests passed, 1 0 0 0 0 0 0 0 failed

The failing test was 4.22 with Step0-DAS_ERROR, thus unrelated to this PR.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @jan-kaspar for master.

It involves the following packages:

CondFormats/CTPPSReadoutObjects
DQM/CTPPS
DataFormats/FEDRawData
EventFilter/CTPPSRawToDigi
RecoCTPPS/Configuration

@perrotta, @ghellwig, @arunhep, @emeschi, @cerminar, @dmitrijus, @cmsbuild, @ggovi, @franzoni, @slava77, @mommsen, @mmusich, @vanbesien, @davidlange6 can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @ghellwig, @apfeiffer1, @tocheng this is something you requested to watch as well.
@Muzaffar, @davidlange6, @smuzaffar you are the release manager for this.

cms-bot commands are listed here #13028

@perrotta
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 10, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/19047/console Started: 2017/04/10 12:51

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@mommsen
Copy link
Contributor

mommsen commented Apr 10, 2017

+1
The scattering of Totem FED ranges is not nice. But I guess there's nothing we can do about it now.

@cmsbuild
Copy link
Contributor

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

Comparison Summary:

  • You potentially added 78 lines to the logs
  • Reco comparison results: 3259 differences found in the comparisons
  • DQMHistoTests: Total files compared: 23
  • DQMHistoTests: Total histograms compared: 1912203
  • DQMHistoTests: Total failures: 43829
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 1868201
  • DQMHistoTests: Total skipped: 173
  • DQMHistoTests: Total Missing objects: 0
  • Checked 94 log files, 14 edm output root files, 23 DQM output files

@jan-kaspar
Copy link
Contributor Author

@perrotta You are perfectly right. A line printing an error message was accidentally removed in an old commit:
CTPPS@0c38042#diff-e01e0047604a3839bb019fa87ee6be52L109
It is now fixed in 6d46b3c. Many thanks for pointing this out.

Also to @slava77 - the same bug is in 8_0_X:
https://github.com/cms-sw/cmssw/blob/CMSSW_8_0_X/EventFilter/CTPPSRawToDigi/src/RawToDigiConverter.cc#L111
and of course it would be better to fix it before the re-reco. Could we bundle this small change with the one we are discussing over the email?

@perrotta
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 25, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/19371/console Started: 2017/04/25 14:22

@slava77
Copy link
Contributor

slava77 commented Apr 25, 2017 via email

@dmitrijus
Copy link
Contributor

+1

@perrotta
Copy link
Contributor

+1
jenkins comparisons show no differences (which is good, as this PR is meant to 2017 data taking)
code looks correct, and requested adjustments were applied

@mommsen
Copy link
Contributor

mommsen commented Apr 25, 2017

+1

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@jan-kaspar
Copy link
Contributor Author

Yes, please go ahead with an update in 80X as well.

Thanks to @forthommel , a back-port of this bug fix has been included in #18461 by 3cceaaa .

@cmsbuild
Copy link
Contributor

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

Comparison Summary:

  • You potentially added 79 lines to the logs
  • Reco comparison results: 69 differences found in the comparisons
  • DQMHistoTests: Total files compared: 23
  • DQMHistoTests: Total histograms compared: 1775104
  • DQMHistoTests: Total failures: 14592
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 1760339
  • DQMHistoTests: Total skipped: 173
  • DQMHistoTests: Total Missing objects: 0
  • Checked 94 log files, 14 edm output root files, 23 DQM output files

@arunhep
Copy link
Contributor

arunhep commented Apr 25, 2017

+1

@davidlange6 davidlange6 merged commit b83b836 into cms-sw:master Apr 27, 2017
@jan-kaspar jan-kaspar deleted the ctpps_strips_update_2017 branch June 23, 2017 18:24
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

10 participants