`BeamSpot` Payload Inspector: introduce payload parameters comparator #37143

mmusich · 2022-03-04T15:24:38Z

PR description:

The goal of this PR is to introduce a new type of class for the BeamSpot Payload Inspector that allows to compare the BeamSpot parameters for two different payloads, either two different IOVs form the same tag, or from two different tags .
This is done via a novel templated DisplayParametersDiff class that will accept both BeamSpotObjects and BeamSpotOnlineObjects data types.
The unit tests of the package CondCore/BeamSpotPlugins are enhanced in order to test the newly added functionality.

PR validation:

Tested with the following commands:

getPayloadData.py \
    --plugin pluginBeamSpot_PayloadInspector \
    --plot plot_BeamSpotParametersDiffSingleTag \
    --tag BeamSpotObjects_PCL_byLumi_v0_prompt \
    --time_type Lumi \
    --iovs '{"start_iov": "1406859487477814", "end_iov": "1488257707672138"}' \
    --db Prod \
    --test ;

and:

getPayloadData.py \
    --plugin pluginBeamSpot_PayloadInspector \
    --plot plot_BeamSpotParametersDiffTwoTags \
    --tag BeamSpotObjects_Realistic25ns_900GeV_2021PilotBeams_v2_mc \
    --time_type Run \
    --iovs '{"start_iov": "1", "end_iov": "1"}' \
    --tagtwo BeamSpotObjects_Realistic25ns_900GeV_2021PilotBeams_v1_mc \
    --iovstwo '{"start_iov": "1", "end_iov": "1"}' \
    --db Prod \
    --test ;

and obtained the following plots:

BeamSpotParametersDiffSingleTag	BeamSpotParametersDiffTwoTags

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

N/A

…amSpotPlugins unit tests

cmsbuild · 2022-03-04T15:33:04Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37143/28682

This PR adds an extra 24KB to repository

cmsbuild · 2022-03-04T15:33:28Z

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

It involves the following packages:

CondCore/BeamSpotPlugins (db)

@cmsbuild, @ggovi, @tvami, @malbouis, @francescobrivio 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

mmusich · 2022-03-04T15:35:31Z

@cmsbuild, please test

…ass BSParamsHelper

…eObjects

cmsbuild · 2022-03-04T20:35:38Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-28be95/22847/summary.html
COMMIT: d20ead3
CMSSW: CMSSW_12_3_X_2022-03-04-1100/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/37143/22847/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: 5 differences found in the comparisons
DQMHistoTests: Total files compared: 49
DQMHistoTests: Total histograms compared: 3987741
DQMHistoTests: Total failures: 8
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3987711
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
Checked 204 log files, 45 edm output root files, 49 DQM output files
TriggerResults: no differences found

tvami · 2022-03-04T23:01:11Z

+db

cmsbuild · 2022-03-04T23:01:33Z

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)

perrotta · 2022-03-05T17:08:08Z

please test

I can hardly believe that the differences in the haloTaggerMVA for patPhotons are originated by this PR, eg:

Let rerun the tests, to see if it is reproducible, at least.

cmsbuild · 2022-03-05T21:00:34Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-28be95/22873/summary.html
COMMIT: d20ead3
CMSSW: CMSSW_12_3_X_2022-03-05-1100/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/37143/22873/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: 49
DQMHistoTests: Total histograms compared: 3987741
DQMHistoTests: Total failures: 7
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 3987711
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.004 KiB( 48 files compared)
DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
Checked 204 log files, 45 edm output root files, 49 DQM output files
TriggerResults: no differences found

perrotta · 2022-03-06T12:03:38Z

So, differences are still there, but they now look different than in the previous comparisons. This is the same wf 136.7611 that was reported above

While having non identical distributions in consecutive runs of the bot tests probably confirms that there is some non reproducibily issue for those patPhotons' haloTaggerMVA, I don't remember having seen it in other PR tests.
@mmusich @tvami do you have any idea whether these distributions can be touched in any way by the updates implemented with this PR?

mmusich · 2022-03-06T14:39:06Z

@perrotta

do you have any idea whether these distributions can be touched in any way by the updates implemented with this PR?

There's no way that changes in these classes can affect anything in any relval.
I think these differences might be caused by some uninitialized variable in #36901.
Also notice that this particular variable started to be included in the reco comparison also recently (see cms-sw/cms-bot#1717)

mmusich · 2022-03-06T16:29:01Z

since this was being held anyway, I profited to push some other improvement.

cmsbuild · 2022-03-06T16:34:22Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37143/28703

This PR adds an extra 24KB to repository

cmsbuild · 2022-03-06T16:34:49Z

Pull request #37143 was updated. can you please check and sign again.

mmusich · 2022-03-06T16:41:02Z

@cmsbuild, please test

cmsbuild · 2022-03-06T20:32:34Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-28be95/22885/summary.html
COMMIT: 8c57f57
CMSSW: CMSSW_12_3_X_2022-03-05-1100/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/37143/22885/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: 1 differences found in the comparisons
DQMHistoTests: Total files compared: 49
DQMHistoTests: Total histograms compared: 3987741
DQMHistoTests: Total failures: 2
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3987717
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
Checked 204 log files, 45 edm output root files, 49 DQM output files
TriggerResults: no differences found

tvami · 2022-03-06T20:35:22Z

+1

qliphy · 2022-03-07T00:38:16Z

+1
differences in the haloTaggerMVA for patPhotons is not related to this PR, and will be fixed later (related to #36901)

mmusich added 3 commits March 4, 2022 16:09

introduce difference plot for BeamSpotObjects parameters

735c990

add difference plot for BeamSpotObjects parameters to the CondCore/Be…

2f850ba

…amSpotPlugins unit tests

miscellaneous improvements to BeamSpotPayloadInspectorHelper

4c4251c

cmsbuild added this to the CMSSW_12_3_X milestone Mar 4, 2022

cmsbuild added code-checks-pending db-pending orp-pending pending-signatures tests-pending labels Mar 4, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Mar 4, 2022

cmsbuild added tests-started and removed tests-pending labels Mar 4, 2022

mmusich added 2 commits March 4, 2022 19:50

improve BeamSpotPayloadInspectorHelper by making use of the helper cl…

333a337

…ass BSParamsHelper

enhance the BeamSpotPayloadInspector unit tests for the BeamSpotOnlin…

8c57f57

…eObjects

cmsbuild added tests-approved and removed tests-started labels Mar 4, 2022

cmsbuild added db-approved fully-signed and removed db-pending pending-signatures labels Mar 4, 2022

cmsbuild added tests-started and removed tests-approved labels Mar 5, 2022

cmsbuild added tests-approved and removed tests-started labels Mar 5, 2022

mmusich force-pushed the BeamSpot_PayloadInspector_ParametersDiff branch from d20ead3 to 8c57f57 Compare March 6, 2022 16:28

cmsbuild added code-checks-pending tests-started and removed tests-approved code-checks-approved labels Mar 6, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Mar 6, 2022

perrotta mentioned this pull request Mar 6, 2022

Implementation of beam halo tagger algorithm for the endcap photons #36901

Merged

cmsbuild added tests-approved and removed tests-started labels Mar 6, 2022

cmsbuild added orp-approved and removed orp-pending labels Mar 7, 2022

cmsbuild merged commit 88392ca into cms-sw:master Mar 7, 2022

jainshilpi mentioned this pull request Mar 7, 2022

Fix for the access of haloMVATaggerVal in the Run 1+2 files #37156

Merged

mmusich deleted the BeamSpot_PayloadInspector_ParametersDiff branch March 7, 2022 11:15

This was referenced Mar 7, 2022

[ROOT626] Updated root to tip of branch v6-26-00-patches cms-sw/cmsdist#7670

Merged

Define HOST_DEVICE_CONSTANT to implement constexpr aggregate host/device constants #37159

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`BeamSpot` Payload Inspector: introduce payload parameters comparator #37143

`BeamSpot` Payload Inspector: introduce payload parameters comparator #37143

mmusich commented Mar 4, 2022

cmsbuild commented Mar 4, 2022

cmsbuild commented Mar 4, 2022

mmusich commented Mar 4, 2022

cmsbuild commented Mar 4, 2022

tvami commented Mar 4, 2022

cmsbuild commented Mar 4, 2022

perrotta commented Mar 5, 2022 •

edited

cmsbuild commented Mar 5, 2022

perrotta commented Mar 6, 2022

mmusich commented Mar 6, 2022 •

edited

mmusich commented Mar 6, 2022

cmsbuild commented Mar 6, 2022

cmsbuild commented Mar 6, 2022

mmusich commented Mar 6, 2022

cmsbuild commented Mar 6, 2022

tvami commented Mar 6, 2022

qliphy commented Mar 7, 2022

BeamSpot Payload Inspector: introduce payload parameters comparator #37143

BeamSpot Payload Inspector: introduce payload parameters comparator #37143

Conversation

mmusich commented Mar 4, 2022

PR description:

PR validation:

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

cmsbuild commented Mar 4, 2022

cmsbuild commented Mar 4, 2022

mmusich commented Mar 4, 2022

cmsbuild commented Mar 4, 2022

Comparison Summary

tvami commented Mar 4, 2022

cmsbuild commented Mar 4, 2022

perrotta commented Mar 5, 2022 • edited

cmsbuild commented Mar 5, 2022

Comparison Summary

perrotta commented Mar 6, 2022

mmusich commented Mar 6, 2022 • edited

mmusich commented Mar 6, 2022

cmsbuild commented Mar 6, 2022

cmsbuild commented Mar 6, 2022

mmusich commented Mar 6, 2022

cmsbuild commented Mar 6, 2022

Comparison Summary

tvami commented Mar 6, 2022

qliphy commented Mar 7, 2022

`BeamSpot` Payload Inspector: introduce payload parameters comparator #37143

`BeamSpot` Payload Inspector: introduce payload parameters comparator #37143

perrotta commented Mar 5, 2022 •

edited

mmusich commented Mar 6, 2022 •

edited