New schema evolution unit test #42825

wddgit · 2023-09-19T17:05:59Z

PR description:

Add a new unit test that verifies the performance of schema evolution algorithms in ROOT. This also tests that StreamerInfo objects are written for classes nested in other classes that have a StreamerInfo.

In the test, an input file is read that contains products containing known values. We verify we can read the product and that the values match values we know were written into the product. The input file was created in a separate process that was built with modified data format classes. This process was prepared manually by editing the code in this pull request and rebuilding. Then that output was saved in the cms-data IOPool-Input repository in GitHub.

The modifications test each of the 12 kinds of schema evolution listed in ROOT's documentation. At the moment, 2 of these fail and the related tests are commented out in this PR. These do not appear to be cases that are important to CMS (both involve raw pointers as data members), but we'll inform ROOT developers about this.

This is what the ROOT documentation says at the current time (9/19/2023, https://root.cern/manual/io/#automatic-schema-evolution):

Automatic schema evolution supports the following scenarios:

Change in the order of data members in the class.
Addition of a data member: the value of the missing member will be left unchanged by the I/O (so usually the value set by the default constructor).
Removal of a data member: the corresponding data is not read.
Move of a data member from a derived class to a base class or vice-versa.
Change of the type of a member if it is a simple type or a pointer to a simple type, including Double32_t and Float16_t. A warning is given in case of loss of precision.
Addition or removal of a base class.
Change of a member type from T* to T or back.
Change of a member type from T* to unique_ptr or back.
Change of a member type from C-style array (such as int[3]) to its std::array counterpart (such as array<int, 3>).
Change from variable-size array and size (such as float *fArray; //[fSize] and int fSize) to std::vector (such as std::vector fArray;).
Change between STL collection types, from / to std::vector, std::queue, std::deque, std::list, std::forward_list, std::set, std::multiset, std::unordered_set, std::unordered_multiset, std::valarray, std::bitset.
Change of STL associative containers, from / to std::map, std::unordered_map, std::multimap, std::unordered_map, std::unordered_multimap std::vector<std::pair<key,value>>.

There is one test case for each of the 12 items listed above (one could imagine writing many test cases for each item above). I could add more test cases if anyone can think of a particular case worth testing.

Currently, the input file was generated with CMSSW_13_2_3. I could add more input files from other releases if requested. We had discussed adding a new input file each time there is a major change in ROOT in the future.

There is also a commented out test for CMSSW_13_0_0. This release has a bug and the test is commented out because it fails for that release. This particular bug is what motivated this PR. See discussion in issues #41246 and #41348. The StreamerInfo test in this PR would have detected that particular bug. This may be useful if we have to do further work to deal with data files that were written by code with the bug.

PR validation:

The new unit test passes.

wddgit · 2023-09-19T17:09:25Z

IOPool/Input/test/SchemaEvolution_test_read_cfg.py

+argv = sys.argv[:]
+if '--' in argv:
+    argv.remove("--")
+args, unknown = parser.parse_known_args(argv)


The part above is going to need to be modified if #42650 gets merged before this one.

@wddgit This part can be modifier now.

cmsbuild · 2023-09-19T17:14:30Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42825/36951

This PR adds an extra 36KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File IOPool/Input/test/BuildFile.xml modified in PR(s): Improve memory usage in ParameterSet #42742

wddgit · 2023-09-19T17:14:55Z

please test with cms-data/IOPool-Input#1

cmsbuild · 2023-09-19T17:15:01Z

A new Pull Request was created by @wddgit (W. David Dagenhart) for master.

It involves the following packages:

DataFormats/TestObjects (core)
IOPool/Input (core)

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

cms-bot commands are listed here

cmsbuild · 2023-09-19T18:04:01Z

-1

Failed Tests: ClangBuild
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-0e0441/34829/summary.html
COMMIT: 3f5bd8d
CMSSW: CMSSW_13_3_X_2023-09-19-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/42825/34829/install.sh to create a dev area with all the needed externals and cmssw changes.

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-0e0441/34829/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-0e0441/34829/git-merge-result

Clang Build

I found compilation warning while trying to compile with clang. Command used:

USER_CUDA_FLAGS='--expt-relaxed-constexpr' USER_CXXFLAGS='-Wno-register -fsyntax-only' scram build -k -j 32 COMPILER='llvm compile'

See details on the summary page.

cmsbuild · 2023-09-19T18:31:07Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42825/36952

This PR adds an extra 36KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File IOPool/Input/test/BuildFile.xml modified in PR(s): Improve memory usage in ParameterSet #42742

cmsbuild · 2023-09-19T18:31:35Z

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

wddgit · 2023-09-19T18:33:56Z

please test with cms-data/IOPool-Input#1

try again, I deleted the unused variable that clang found, squashed and force pushed

cmsbuild · 2023-10-13T21:03:51Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42825/37191

This PR adds an extra 20KB to repository

cmsbuild · 2023-10-13T21:04:15Z

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

wddgit · 2023-10-13T21:04:46Z

please test with cms-data/IOPool-Input#1

The last commit now adjusts the test to use the new cmsRun command line interface recently merged. Also rebased. Nothing else was changed in the last force push of modified code.

cmsbuild · 2023-10-14T04:12:22Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-0e0441/35190/summary.html
COMMIT: 14a84bd
CMSSW: CMSSW_13_3_X_2023-10-13-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/42825/35190/install.sh to create a dev area with all the needed externals and cmssw changes.

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

@abdoulline HCAL unpacker: adding more protection against corrupted data #43011

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-0e0441/35190/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-0e0441/35190/git-merge-result

Comparison Summary

Summary:

You potentially removed 5 lines from the logs
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 50
DQMHistoTests: Total histograms compared: 3356920
DQMHistoTests: Total failures: 0
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3356898
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 49 files compared)
Checked 214 log files, 167 edm output root files, 50 DQM output files
TriggerResults: no differences found

makortel · 2023-10-15T19:38:42Z

+core

cmsbuild · 2023-10-15T19:39:07Z

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

rappoccio · 2023-10-16T16:19:13Z

+1

cmsbuild added this to the CMSSW_13_3_X milestone Sep 19, 2023

cmsbuild added core-pending pending-signatures tests-pending orp-pending code-checks-pending labels Sep 19, 2023

wddgit commented Sep 19, 2023

View reviewed changes

cmsbuild added tests-started requires-external code-checks-approved and removed tests-pending code-checks-pending labels Sep 19, 2023

cmsbuild added tests-rejected and removed tests-started labels Sep 19, 2023

wddgit force-pushed the testSchemaEvolution branch from 3f5bd8d to 517288c Compare September 19, 2023 18:23

cmsbuild added tests-pending code-checks-pending and removed tests-rejected requires-external code-checks-approved labels Sep 19, 2023

cmsbuild added code-checks-approved and removed code-checks-pending labels Sep 19, 2023

cmsbuild removed the tests-pending label Sep 19, 2023

cmsbuild added tests-pending code-checks-pending and removed tests-approved requires-external code-checks-approved labels Oct 13, 2023

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 13, 2023

cmsbuild added tests-started requires-external and removed tests-pending labels Oct 13, 2023

cmsbuild added tests-approved and removed tests-started labels Oct 14, 2023

cmsbuild added core-approved fully-signed and removed core-pending pending-signatures labels Oct 15, 2023

cmsbuild added orp-approved and removed orp-pending labels Oct 16, 2023

cmsbuild merged commit f18da9b into cms-sw:master Oct 16, 2023
11 checks passed

antoniovilela mentioned this pull request Oct 16, 2023

Update tag for IOPool-Input to V00-02-00 cms-sw/cmsdist#8762

Merged

This was referenced Oct 16, 2023

[do not merge][ASAN] Build with -O0 flag cms-sw/cmsdist#8761

Closed

[G4] Update geant4 to 11.1.ref09 cms-sw/cmsdist#8741

Merged

wddgit deleted the testSchemaEvolution branch March 11, 2024 14:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New schema evolution unit test #42825

New schema evolution unit test #42825

wddgit commented Sep 19, 2023

wddgit Sep 19, 2023

makortel Oct 13, 2023

cmsbuild commented Sep 19, 2023

wddgit commented Sep 19, 2023

cmsbuild commented Sep 19, 2023

cmsbuild commented Sep 19, 2023

cmsbuild commented Sep 19, 2023

cmsbuild commented Sep 19, 2023

wddgit commented Sep 19, 2023

cmsbuild commented Oct 13, 2023

cmsbuild commented Oct 13, 2023

wddgit commented Oct 13, 2023

cmsbuild commented Oct 14, 2023

makortel commented Oct 15, 2023

cmsbuild commented Oct 15, 2023

rappoccio commented Oct 16, 2023

New schema evolution unit test #42825

New schema evolution unit test #42825

Conversation

wddgit commented Sep 19, 2023

PR description:

PR validation:

wddgit Sep 19, 2023

Choose a reason for hiding this comment

makortel Oct 13, 2023

Choose a reason for hiding this comment

cmsbuild commented Sep 19, 2023

wddgit commented Sep 19, 2023

cmsbuild commented Sep 19, 2023

cmsbuild commented Sep 19, 2023

Clang Build

cmsbuild commented Sep 19, 2023

cmsbuild commented Sep 19, 2023

wddgit commented Sep 19, 2023

cmsbuild commented Oct 13, 2023

cmsbuild commented Oct 13, 2023

wddgit commented Oct 13, 2023

cmsbuild commented Oct 14, 2023

Comparison Summary

makortel commented Oct 15, 2023

cmsbuild commented Oct 15, 2023

rappoccio commented Oct 16, 2023