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

First version of Payload Inspector plots for DropBoxMetadata #36520

Merged
merged 1 commit into from Dec 21, 2021

Conversation

tlampen
Copy link
Contributor

@tlampen tlampen commented Dec 16, 2021

PR description:

This PR introduces new plots for the Payload Inspector for the class DropBoxMetadata as well as a script for testing. These plots can 1) display content of a DropBoxMetadata tag, 2) compare two IOVs of one tag, and 3) compare two tags. Examples of the plots are attached.

This PR continues from the infrastructural changes introduced in #35956 . This PR however does not fully exploit the new intrastructure, since it is useful to quickly make these new tools available.

HLTPlugins/plugins/AlCaRecoTriggerBits_PayloadInspector.cc was used as a source of inspiration for this work.

DropBoxMetadata is describe in this TWiki page: https://twiki.cern.ch/twiki/bin/view/CMS/ConditionUploader#Metadata_file .

PR validation:

PR passes the introduced test.

if this PR is a backport please specify the original PR and why you need to backport that PR:

No backport foreseen.


DropBoxMetadata_Display
DropBoxMetadata_Compare
DropBoxMetadata_CompareTwoTags

@tlampen
Copy link
Contributor Author

tlampen commented Dec 16, 2021

assign alca

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36520/27407

  • This PR adds an extra 20KB to repository

Code check has found code style and quality issues which could be resolved by applying following patch(s)

@francescobrivio
Copy link
Contributor

assign alca

@@ -175,9 +217,15 @@ namespace {

l_theDisplay.printDiffWithMetadata(f_theRecordMap);

TCanvas canvas("Canv", "Canv", 1200, 100 * std::max(f_records.size(), l_records.size()));
std::string fileName(this->m_imageFileName);
Copy link
Contributor

Choose a reason for hiding this comment

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

I am afraid you need to percolate this within DBMetaDataPlotDisplay and use it to save the result png file otherwise the PayloadInspector instance of the ConDB Browser won't be able to fetch the plot to be displayed.

@@ -107,8 +145,12 @@ namespace {
DBoxMetadataHelper::DBMetaDataTableDisplay theDisplay(theRecordMap);
theDisplay.printMetaDatas();

std::string fileName(m_imageFileName);
Copy link
Contributor

Choose a reason for hiding this comment

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

I am afraid you need to percolate this within DBMetaDataPlotDisplay and use it to save the result png file otherwise the PayloadInspector instance of the ConDB Browser won't be able to fetch the plot to be displayed.

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 for pointing this out. I'll make the change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I hope this approach works.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36520/27444

  • This PR adds an extra 20KB to repository

@cmsbuild
Copy link
Contributor

New categories assigned: alca

@yuanchao,@francescobrivio,@malbouis,@tvami you have been requested to review this Pull request/Issue and eventually sign? Thanks

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @tlampen (Tapio Lampén) for master.

It involves the following packages:

  • CondCore/PhysicsToolsPlugins (db)

@malbouis, @yuanchao, @cmsbuild, @ggovi, @francescobrivio, @tvami can you please review it and eventually sign? Thanks.
@mmusich this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36520/27447

  • This PR adds an extra 20KB to repository

Code check has found code style and quality issues which could be resolved by applying following patch(s)

@tvami
Copy link
Contributor

tvami commented Dec 17, 2021

@tlampen please do code-checks, thanks!

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36520/27461

  • This PR adds an extra 20KB to repository

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36520/27488

  • This PR adds an extra 20KB to repository

@cmsbuild
Copy link
Contributor

Pull request #36520 was updated. @malbouis, @yuanchao, @cmsbuild, @ggovi, @francescobrivio, @tvami can you please check and sign again.

@tlampen
Copy link
Contributor Author

tlampen commented Dec 20, 2021

The plots can be created with the script test_DropBoxMetadata_PayloadInspector.sh . I don't know how the plots can be saved to the unit test directory, it would indeed be useful to have them there.

@yuanchao
Copy link
Contributor

@cmsbuild please test

@mmusich
Copy link
Contributor

mmusich commented Dec 20, 2021

I don't know how the plots can be saved to the unit test directory, it would indeed be useful to have them there.

in principle this unit test:

<bin file="test_DropBoxMetadata_PayloadInspector.cpp" name="test_DropBoxMetadata_PayloadInspector">

produces plots both in IBs and PR tests, though I think the bot is setup to ignore anything else which is not log.
(BTW: you can produce them locally with scram b runtests_test_DropBoxMetadata_PayloadInspector)

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-5436d8/21400/summary.html
COMMIT: 1130996
CMSSW: CMSSW_12_3_X_2021-12-20-1100/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/36520/21400/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: 2 differences found in the comparisons
  • DQMHistoTests: Total files compared: 43
  • DQMHistoTests: Total histograms compared: 3461688
  • DQMHistoTests: Total failures: 6
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3461660
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 42 files compared)
  • Checked 181 log files, 42 edm output root files, 43 DQM output files
  • TriggerResults: no differences found

@tvami
Copy link
Contributor

tvami commented Dec 20, 2021

+1

  • tests pass
  • code changes in line with the description

@cmsbuild
Copy link
Contributor

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

@qliphy
Copy link
Contributor

qliphy commented Dec 21, 2021

+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

7 participants