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

Drop type specs for DQM/Integration python files #34170

Merged
merged 7 commits into from Jun 22, 2021

Conversation

jfernan2
Copy link
Contributor

PR description:

Drop type specs in DQM/Integration/python configurations files, which drive the Online DQM clients at P5

Since with the change the mis-spelled variable name would give error, some "hidden" non-existing/wrong parameters or not actually declared in the original cloned module, have been identified in the process

Hence some extra DQM packages have been updated to make it work and some other parameters have been commented out since they do not actually exist in the plugin definition

PR validation:

Unit Tests are reproduced

process.MonitorTrackResiduals_gentk.TrackProducer = cms.string('initialStepTracksPreSplitting')
process.TrackMon_gentk.TrackProducer = cms.InputTag('initialStepTracksPreSplitting')
process.TrackMon_gentk.allTrackProducer = cms.InputTag('initialStepTracksPreSplitting')
# process.MonitorTrackResiduals_gentk.TrackProducer = 'initialStepTracksPreSplitting' ##DOES NOT EXIST ON DQM/TrackerMonitorTrack/src/MonitorTrackResiduals.cc
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mtosi @mmusich if you could please confirm? Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the change per se looks OK (please remove the commented code as well), though the module is not cloned here while in the PR description there's written:

... "hidden" non-existing/wrong parameters or not actually declared in the original cloned module, have been identified in the process

process.SiStripMonitorTrack_gentk.TrackProducer = 'initialStepTracksPreSplitting'

process.SiStripSources_TrkReco = cms.Sequence(process.SiStripMonitorTrack_gentk*process.MonitorTrackResiduals_gentk*process.TrackMon_gentk)

### STRIP
process.load("DQM.SiStripMonitorClient.SiStripClientConfigP5_cff")
process.SiStripAnalyser.UseGoodTracks = cms.untracked.bool(True)
# process.SiStripAnalyser.UseGoodTracks = True #DOES NOT EXIST ON DQM/SiStripMonitorClient/plugins/SiStripAnalyser.cc
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mtosi @mmusich if you could please confirm? Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the change per se looks OK (please remove the commented code as well), though the module is not cloned here while in the PR description there's written:

... "hidden" non-existing/wrong parameters or not actually declared in the original cloned module, have been identified in the process

by the way it seems there is a parameter with the same name configured here:

UseGoodTracks = cms.untracked.bool(True),

and it doesn't seem to be registered in any source code:

https://cmssdt.cern.ch/dxr/CMSSW/search?q=UseGoodTracks&case=true&redirect=true

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@jfernan2
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34170/23378

  • This PR adds an extra 80KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @jfernan2 for master.

It involves the following packages:

DQM/BeamMonitor
DQM/CSCMonitorModule
DQM/Integration
DQM/PixelLumi
DQM/SiStripMonitorClient
DQM/TrackingMonitorClient
DQM/TrackingMonitorSource

@andrius-k, @kmaeshima, @ErnestaP, @ahmad3213, @jfernan2, @rvenditti can you please review it and eventually sign? Thanks.
@mtosi, @arossi83, @barvic, @batinkov, @hdelanno, @battibass, @makortel, @sroychow, @JanFSchulte, @fioriNTU, @jandrea, @ptcox, @idebruyn, @threus, @venturia this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

@cmsbuild
Copy link
Contributor

-1

Failed Tests: UnitTests
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3ec48b/16084/summary.html
COMMIT: ca1ffff
CMSSW: CMSSW_12_0_X_2021-06-17-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/34170/16084/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found errors in the following unit tests:

---> test TestDQMOnlineClient-dt_dqm_sourceclient had ERRORS
---> test TestDQMOnlineClient-dt4ml_dqm_sourceclient had ERRORS

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 38
  • DQMHistoTests: Total histograms compared: 2871916
  • DQMHistoTests: Total failures: 6
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 2871887
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -0.004 KiB( 37 files compared)
  • DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
  • Checked 160 log files, 37 edm output root files, 38 DQM output files
  • TriggerResults: no differences found

@jfernan2
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

Pull request #34170 was updated. @andrius-k, @kmaeshima, @ErnestaP, @ahmad3213, @jfernan2, @rvenditti can you please check and sign again.

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3ec48b/16098/summary.html
COMMIT: cb0a24c
CMSSW: CMSSW_12_0_X_2021-06-17-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/34170/16098/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 38
  • DQMHistoTests: Total histograms compared: 2871916
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2871893
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 37 files compared)
  • Checked 160 log files, 37 edm output root files, 38 DQM output files
  • TriggerResults: no differences found

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34170/23409

@cmsbuild
Copy link
Contributor

Pull request #34170 was updated. @andrius-k, @kmaeshima, @ErnestaP, @ahmad3213, @cmsbuild, @jfernan2, @rvenditti can you please check and sign again.

@mmusich
Copy link
Contributor

mmusich commented Jun 21, 2021

@jfernan2 I was wondering if a general tool to compare the output of the online DQM clients exists (ad if yes it can be made part of the integration tests)

@jfernan2
Copy link
Contributor Author

jfernan2 commented Jun 21, 2021

Thanks for the suggestion @mmusich !
The tool compareHistograms.py and compareDQMOutput.py exist in CMSSW and these are what I have used to compared manually the Online root files produced by this PR with baseline.
However, making this comparison part of the integration tests would imply changes in the DQM test GUI which hosts this kind of comparisons, since Online root files are not properly displayed due to ROOT folders, just RelVal or Offline ones...
With the transition to the new DQM GUI, this test may be easier to implement

@jfernan2
Copy link
Contributor Author

+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. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3ec48b/16143/summary.html
COMMIT: 022eff0
CMSSW: CMSSW_12_0_X_2021-06-21-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/34170/16143/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 38
  • DQMHistoTests: Total histograms compared: 2785631
  • DQMHistoTests: Total failures: 7
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2785602
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 37 files compared)
  • Checked 160 log files, 37 edm output root files, 38 DQM output files
  • TriggerResults: no differences found

@qliphy
Copy link
Contributor

qliphy commented Jun 22, 2021

+1

@cmsbuild cmsbuild merged commit 51c6c8e into cms-sw:master Jun 22, 2021
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

4 participants