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

[10.2.X] Bug fixed in SiStripBadComponentInfo #23554

Merged
merged 2 commits into from
Jun 12, 2018

Conversation

mmusich
Copy link
Contributor

@mmusich mmusich commented Jun 11, 2018

Greetings,
this PR implements a bug-fix in SiStripBadComponentInfo.cc where the bad fiber histogram never got filled (same a proposed in #23491).
In addition for what concerns the addition of bad components from FED errors, this PR is implements a fix via cloning the siStripQualityESProducer and then modifying the attributes instead of just changing the attributes from the imported module.
Tested against the regular workflow (non-DQM based) and found perfect agreement.
@suchandradutta @fioriNTU @jandrea @arossi83

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@mmusich
Copy link
Contributor Author

mmusich commented Jun 11, 2018

type bug-fix

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @mmusich (Marco Musich) for master.

It involves the following packages:

DQM/SiStripMonitorClient

@vazzolini, @kmaeshima, @dmitrijus, @cmsbuild, @jfernan2, @vanbesien can you please review it and eventually sign? Thanks.
@hdelanno, @fioriNTU, @jandrea, @idebruyn, @threus, @venturia 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

@davidlange6
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 11, 2018

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/28624/console Started: 2018/06/11 16:44

@dmitrijus
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next master IBs after it passes the integration tests. 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)

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

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

Comparison Summary:

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

@fabiocos
Copy link
Contributor

@mmusich I see that the differences are affecting the SiStrip histograms, are they what is expected?

@mmusich
Copy link
Contributor Author

mmusich commented Jun 12, 2018

@fabiocos, as the histograms touched by this PR are located in SiStrip/Run summary/EventInfo/BadComponentContents/ it is expected that the differences show up in this folder as it is the case e.g. for:

I also checked that the numbers obtained in the DQM plots (e.g. for worfklow 10042.0) are the same as the ones obtained using the "classical" bad components merging (see cfg here):

-----------------
Global Info
-----------------
BadComponent 	   Modules 	Fibers 	Apvs	Strips
----------------------------------------------------------------
Tracker:		508	1307	2654	358119

TIB:			222	557	1123	147140
TID:			39	116	237	30519
TOB:			122	291	586	78385
TEC:			125	343	708	102075

TIB Layer 1 :		55	168	337	43709
TIB Layer 2 :		48	147	295	38346
TIB Layer 3 :		47	96	195	25693
TIB Layer 4 :		72	146	296	39392

TID+ Disk 1 :		3	9	18	2308
TID+ Disk 2 :		6	16	34	4353
TID+ Disk 3 :		1	3	7	898
TID- Disk 1 :		7	21	43	5549
TID- Disk 2 :		22	67	134	17281
TID- Disk 3 :		0	0	1	130

TOB Layer 1 :		12	26	52	7192
TOB Layer 2 :		18	41	83	11120
TOB Layer 3 :		24	56	112	14579
TOB Layer 4 :		48	97	194	25727
TOB Layer 5 :		13	45	91	12233
TOB Layer 6 :		7	26	54	7534

TEC+ Disk 1 :		16	41	84	11860
TEC+ Disk 2 :		2	6	12	2947
TEC+ Disk 3 :		4	8	18	3136
TEC+ Disk 4 :		3	9	19	3163
TEC+ Disk 5 :		7	22	45	6241
TEC+ Disk 6 :		12	34	68	8963
TEC+ Disk 7 :		17	44	89	11621
TEC+ Disk 8 :		11	23	47	6519
TEC+ Disk 9 :		6	13	26	3921
TEC- Disk 1 :		1	6	14	2633
TEC- Disk 2 :		10	31	66	8904
TEC- Disk 3 :		2	5	10	2760
TEC- Disk 4 :		0	3	9	1623
TEC- Disk 5 :		1	6	13	2059
TEC- Disk 6 :		9	29	60	8429
TEC- Disk 7 :		15	40	81	10680
TEC- Disk 8 :		4	11	22	2999
TEC- Disk 9 :		5	12	25	3617

----------------------------------------------------------------

To be compared with:

badapv
badfiber
badstrip

@fabiocos
Copy link
Contributor

@mmusich thanks for the confirmation

@fabiocos
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 792e0a6 into cms-sw:master Jun 12, 2018
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.

5 participants