a first unit test for backward compatibility of Scouting data formats #41093

missirol · 2023-03-17T22:42:29Z

PR description:

This PR adds a refined version of the unit test described in #41040 (comment) [*]. The aim of the test is to fail whenever non-backward-compatible changes are introduced in the Scouting data formats.

The implementation of the test could certainly be improved (one suggestion is in #41040 (comment)), but I decided to open a draft PR because I think the test in this PR is better than nothing. (I plan to address #41040 (comment), but I don't have an ETA for that.)

[*]

More precisely, there are two unit tests, one for the Run-2 Scouting data formats ("Scouting*"), and one for the Run-3 ones ("Run3Scouting*"). The two tests have the same structure:

a 1st step reads O(100) events from an existing EDM file, and writes the Scouting collections to disk (this first step should fail if non-backward-compatible changes are made to the data formats);
a 2nd step uses FWLite to print to stdout one entry/object for each of the Scouting data formats being tested. To check that the correct values are read, the test compares the output of step-2 to a reference in the form of a text file. This is currently done for 1 object per data format. It might be necessary to check more than 1 object (and/or more than 1 event), because Add additional track variables to the Run 3 scouting electron collection for low pT electrons. #41025 (comment) showed that some issues can only be spotted when checking multiple objects/events.

PR validation:

The new unit tests pass locally.

If this PR is a backport, please specify the original PR and why you need to backport that PR. If this PR will be backported, please specify to which release cycle the backport is meant for:

If accepted, it would be good to backport it to at least CMSSW_13_0_X.

cmsbuild · 2023-03-17T22:49:03Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41093/34712

This PR adds an extra 24KB to repository

cmsbuild · 2023-03-17T22:49:29Z

A new Pull Request was created by @missirol (Marino Missiroli) for master.

It involves the following packages:

DataFormats/Scouting (core)

@cmsbuild, @smuzaffar, @Dr15Jones, @makortel can you please review it and eventually sign? Thanks.
@rovere this is something you requested to watch as well.
@perrotta, @dpiparo, @rappoccio you are the release manager for this.

cms-bot commands are listed here

makortel · 2023-03-18T05:56:34Z

@cmsbuild, please test

missirol

A couple of typos I'll have to fix.

missirol · 2023-03-18T06:32:19Z

DataFormats/Scouting/test/scoutingCollectionsDumper.py

+            ('vector<Run3ScoutingVertex>', 'hltScoutingPrimaryVertexPacker:primaryVtx')
+        ]
+    else:
+        raise RuntimeError(f'getScoutingProductsList -- invalid key (must be "Scouting", "Run3Scouting"): "{key}"')


Suggested change

raise RuntimeError(f'getScoutingProductsList -- invalid key (must be "Scouting", "Run3Scouting"): "{key}"')

raise RuntimeError(f'getScoutingProductsList -- invalid key (must be "Scouting", or "Run3Scouting"): "{key}"')

missirol · 2023-03-18T06:34:53Z

DataFormats/Scouting/test/scoutingCollectionsIO_cfg.py

+parser.add_argument('-n', '--maxEvents', type = int, help = 'Max number of input events to be processed',
+                    default = 10)
+
+parser.add_argument('--skipEvents', type = int, help = 'Number of input events to skipped',


Suggested change

parser.add_argument('--skipEvents', type = int, help = 'Number of input events to skipped',

parser.add_argument('--skipEvents', type = int, help = 'Number of input events to be skipped',

cmsbuild · 2023-03-18T08:18:52Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4d50da/31391/summary.html
COMMIT: a790a14
CMSSW: CMSSW_13_1_X_2023-03-17-2300/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/41093/31391/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially removed 5 lines from the logs
Reco comparison results: 12 differences found in the comparisons
DQMHistoTests: Total files compared: 49
DQMHistoTests: Total histograms compared: 3550882
DQMHistoTests: Total failures: 6
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3550854
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
Checked 213 log files, 164 edm output root files, 49 DQM output files
TriggerResults: no differences found

cmsbuild · 2023-03-18T21:48:37Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41093/34722

This PR adds an extra 16KB to repository

cmsbuild · 2023-03-18T21:49:05Z

Pull request #41093 was updated. @cmsbuild, @smuzaffar, @Dr15Jones, @makortel can you please check and sign again.

missirol · 2023-03-18T21:49:41Z

please test

(Only fixed a few typos.)

missirol · 2023-03-22T19:15:10Z

I’m wondering how we proceed with this PR. From my point of view, it could be integrated (unfortunately, I haven’t found time yet to work on the ‘EDAnalyzer version’ of the test).

I suppose these tests are better than nothing, so maybe it would be beneficial to get the tests in, and improve later.

Okay, I un-drafted the PR, and the last push integrates #41093 (comment). "Better than nothing" is indeed in the PR description. I would also backport this to 13_0_X, since that will be the data-taking release for a while (although having the unit test in master is probably sufficient to catch non-bckwd-compatible changes).

It might be necessary to check more than 1 object (and/or more than 1 event), because Add additional track variables to the Run 3 scouting electron collection for low pT electrons. #41025 (comment) showed that some issues can only be spotted when checking multiple objects/events.

This is also not addressed in this PR, and should be improved in the future.

cmsbuild · 2023-03-22T19:15:57Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41093/34793

This PR adds an extra 12KB to repository

cmsbuild · 2023-03-22T19:16:23Z

Pull request #41093 was updated. @cmsbuild, @smuzaffar, @Dr15Jones, @makortel can you please check and sign again.

makortel · 2023-03-22T19:25:25Z

@cmsbuild, please test

cmsbuild · 2023-03-22T21:55:06Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4d50da/31524/summary.html
COMMIT: 48774f1
CMSSW: CMSSW_13_1_X_2023-03-22-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/41093/31524/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially added 15 lines to the logs
Reco comparison results: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 49
DQMHistoTests: Total histograms compared: 3552750
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3552727
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
Checked 213 log files, 164 edm output root files, 49 DQM output files
TriggerResults: no differences found

makortel · 2023-03-23T18:41:39Z

Another incarnation of #39803

makortel · 2023-03-23T18:43:04Z

+1

cmsbuild · 2023-03-23T18:43:31Z

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

perrotta · 2023-03-24T07:50:25Z

+1

cmsbuild added this to the CMSSW_13_1_X milestone Mar 17, 2023

cmsbuild added core-pending pending-signatures tests-pending orp-pending code-checks-pending labels Mar 17, 2023

cmsbuild added code-checks-approved and removed code-checks-pending labels Mar 17, 2023

cmsbuild added tests-started and removed tests-pending labels Mar 18, 2023

missirol commented Mar 18, 2023

View reviewed changes

perrotta mentioned this pull request Mar 18, 2023

(13_0_X) Add additional track variables to the Run 3 scouting electron collection for low pT electrons. #41035

Merged

cmsbuild added tests-approved and removed tests-started labels Mar 18, 2023

missirol force-pushed the devel_testDataFormatsScouting branch from a790a14 to eb36eaf Compare March 18, 2023 21:42

cmsbuild added tests-pending code-checks-pending and removed tests-approved code-checks-approved labels Mar 18, 2023

cmsbuild added code-checks-approved and removed code-checks-pending labels Mar 18, 2023

cmsbuild added tests-started and removed tests-pending labels Mar 18, 2023

cmsbuild added the code-checks-pending label Mar 22, 2023

missirol changed the title ~~[RFC] unit test for backward compatibility of Scouting data formats~~ a first unit test for backward compatibility of Scouting data formats Mar 22, 2023

missirol marked this pull request as ready for review March 22, 2023 19:09

cmsbuild added code-checks-approved and removed code-checks-pending labels Mar 22, 2023

cmsbuild added tests-started and removed tests-pending labels Mar 22, 2023

cmsbuild added tests-approved and removed tests-started labels Mar 22, 2023

cmsbuild added core-approved fully-signed and removed core-pending pending-signatures labels Mar 23, 2023

cmsbuild added orp-approved and removed orp-pending labels Mar 24, 2023

cmsbuild merged commit 4f6b1f8 into cms-sw:master Mar 24, 2023

missirol mentioned this pull request Mar 24, 2023

a first unit test for backward compatibility of Scouting data formats [13_0_X] #41168

Merged

cmsbuild mentioned this pull request Mar 24, 2023

Fix build failure in DBG_X #41169

Merged

iarspider mentioned this pull request Mar 25, 2023

[PowerPC] Fix unit tests in DataFormats/Scouting #41189

Closed

missirol deleted the devel_testDataFormatsScouting branch March 30, 2023 09:46

missirol mentioned this pull request May 11, 2023

Add unit test for trigger::TriggerEvent format #41631

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

a first unit test for backward compatibility of Scouting data formats #41093

a first unit test for backward compatibility of Scouting data formats #41093

missirol commented Mar 17, 2023

cmsbuild commented Mar 17, 2023

cmsbuild commented Mar 17, 2023

makortel commented Mar 18, 2023

missirol left a comment

missirol Mar 18, 2023

missirol Mar 18, 2023

cmsbuild commented Mar 18, 2023

cmsbuild commented Mar 18, 2023

cmsbuild commented Mar 18, 2023

missirol commented Mar 18, 2023

missirol commented Mar 22, 2023 •

edited

cmsbuild commented Mar 22, 2023

cmsbuild commented Mar 22, 2023

makortel commented Mar 22, 2023

cmsbuild commented Mar 22, 2023

makortel commented Mar 23, 2023

makortel commented Mar 23, 2023

cmsbuild commented Mar 23, 2023

perrotta commented Mar 24, 2023

	raise RuntimeError(f'getScoutingProductsList -- invalid key (must be "Scouting", "Run3Scouting"): "{key}"')
	raise RuntimeError(f'getScoutingProductsList -- invalid key (must be "Scouting", or "Run3Scouting"): "{key}"')

	parser.add_argument('--skipEvents', type = int, help = 'Number of input events to skipped',
	parser.add_argument('--skipEvents', type = int, help = 'Number of input events to be skipped',

a first unit test for backward compatibility of Scouting data formats #41093

a first unit test for backward compatibility of Scouting data formats #41093

Conversation

missirol commented Mar 17, 2023

PR description:

PR validation:

If this PR is a backport, please specify the original PR and why you need to backport that PR. If this PR will be backported, please specify to which release cycle the backport is meant for:

cmsbuild commented Mar 17, 2023

cmsbuild commented Mar 17, 2023

makortel commented Mar 18, 2023

missirol left a comment

Choose a reason for hiding this comment

missirol Mar 18, 2023

Choose a reason for hiding this comment

missirol Mar 18, 2023

Choose a reason for hiding this comment

cmsbuild commented Mar 18, 2023

Comparison Summary

cmsbuild commented Mar 18, 2023

cmsbuild commented Mar 18, 2023

missirol commented Mar 18, 2023

missirol commented Mar 22, 2023 • edited

cmsbuild commented Mar 22, 2023

cmsbuild commented Mar 22, 2023

makortel commented Mar 22, 2023

cmsbuild commented Mar 22, 2023

Comparison Summary

makortel commented Mar 23, 2023

makortel commented Mar 23, 2023

cmsbuild commented Mar 23, 2023

perrotta commented Mar 24, 2023

missirol commented Mar 22, 2023 •

edited