APE measurement tool: Resolved python3 compatibility issues, added unit test functionality, removed dependency on other package

[mteroerd]

PR description:

autoSubmitter.py did not run on recent CMSSW releases since ROOT does not work with python2, while python3 execution would throw an error from using the output of "check_output" without decode().

Some additional small changes to remove redundancies were applied, as well as a change to the disk space requested by condor jobs (the target input file size is 350MB, but files can be larger than 400MB anyway) and some formatting changes. Also, the method names now adhere to the CMS naming rules.

Additionally, added unit test functionality and removed the need to add the package Alignment/TrackerAlignment whenever a measurement is to be performed.

PR validation:

I executed the program with an example configuration and made sure it ran as expected. All changes should be transparent.
Also, a unit test can now be run to validate the PR.

If this PR will be backported please specify to which release cycle the backport is meant for:

Will be backported to 13_0_X and 13_1_X

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42012/35973

• This PR adds an extra 24KB to repository

[cmsbuild]

A new Pull Request was created by @mteroerd (Marius Teroerde) for master.

It involves the following packages:

• Alignment/APEEstimation (alca)

@cmsbuild, @tvami, @saumyaphor4252, @francescobrivio can you please review it and eventually sign? Thanks.
@mmusich, @tocheng, @adewit 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

[mmusich]

autoSubmitter.py did not run on recent CMSSW releases since ROOT does not work with python2, while python3 execution would throw an error from using the output of "check_output" without decode().

what about adding a unit test, to avoid this gets broken again in the future?

[mteroerd]

what about adding a unit test, to avoid this gets broken again in the future?

I am not sure if it is worth the effort right now, since at some point this should get rewritten to something that uses dagman, in preparation of maybe including APE determination in the PCL. But if one were to add a unit test, one would also have to include the data skimming step if it's supposed to be a meaningful test.

[mmusich]

But if one were to add a unit test, one would also have to include the data skimming step if it's supposed to be a meaningful test.

then please do, it should not be much effort!
Incidentally there is no way to test this PR without it.

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42012/35992

• This PR adds an extra 32KB to repository

[cmsbuild]

Pull request #42012 was updated. @cmsbuild, @tvami, @saumyaphor4252, @francescobrivio can you please check and sign again.

[mmusich]

@cmsbuild, please test

[cmsbuild]

-1

Failed Tests: UnitTests
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ccb048/33271/summary.html
COMMIT: 580d39b
CMSSW: CMSSW_13_2_X_2023-06-19-2300/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/42012/33271/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 ApeTest had ERRORS

Comparison Summary

Summary:

• You potentially removed 9 lines from the logs
• Reco comparison results: 12 differences found in the comparisons
• DQMHistoTests: Total files compared: 48
• DQMHistoTests: Total histograms compared: 3197958
• DQMHistoTests: Total failures: 9
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 3197927
• DQMHistoTests: Total skipped: 22
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
• Checked 207 log files, 159 edm output root files, 48 DQM output files
• TriggerResults: no differences found

[cmsbuild]

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ccb048/33329/summary.html
COMMIT: 40ed46f
CMSSW: CMSSW_13_2_X_2023-06-21-2300/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/42012/33329/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

• You potentially removed 12 lines from the logs
• Reco comparison results: 2 differences found in the comparisons
• DQMHistoTests: Total files compared: 48
• DQMHistoTests: Total histograms compared: 3200270
• DQMHistoTests: Total failures: 3
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 3200245
• DQMHistoTests: Total skipped: 22
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
• Checked 207 log files, 159 edm output root files, 48 DQM output files
• TriggerResults: no differences found

[mmusich]

@cms-sw/alca-l2 please have a look. This needs to be backported.

[francescobrivio]

+1

• PR according to description
• new unitTest is successful

[cmsbuild]

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]

+1

[perrotta]

@mteroerd I notice that when running the APE unit tests in UBSAN IBs the following error message show up (see for example https://cmssdt.cern.ch/SDT/cgi-bin/logreader/el8_amd64_gcc11/CMSSW_13_2_UBSAN_X_2023-06-28-2300/unitTestLogs/Alignment/APEEstimation#/):

===== Test "ApeTest" ====
 TESTING data set skimming
cmsRun /data/cmsbld/jenkins/workspace/ib-run-qa/CMSSW_13_2_UBSAN_X_2023-06-28-2300/src/Alignment/APEEstimation/test/SkimProducer/skimProducer_cfg.py sample=UnitTest
UnitTest: Input sample:  UnitTest
UnitTest: Using output name MC_UnitTest_TkAlMuonIsolated.root
UnitTest: Using global tag 131X_mcRun3_2022_design_v2
UnitTest: # MSG-i trackselectionRefitting:  g4Refitting=False
UnitTest: /data/cmsbld/jenkins/workspace/build-any-ib/w/el8_amd64_gcc11/external/gcc/11.4.1-30ebdc301ebd200f2ae0e3d880258e65/include/c++/11.4.1/future:212:62: runtime error: member call on address 0x147bad9a5560 which does not point to an object of type '_Result_base'
UnitTest: 0x147bad9a5560: note: object has invalid vptr
UnitTest:  7b 14 00 00  b8 ba ec a6 7b 14 00 00  00 00 00 00 00 00 00 00  8c 5e 19 a7 7b 14 00 00  00 00 00 00
UnitTest:               ^~~~~~~~~~~~~~~~~~~~~~~
UnitTest:               invalid vptr
UnitTest:     #0 0x147bac7c858a in std::__future_base::_Result_base::_Deleter::operator()(std::__future_base::_Result_base*) const /data/cmsbld/jenkins/workspace/build-any-ib/w/el8_amd64_gcc11/external/gcc/11.4.1-30ebdc301ebd200f2ae0e3d880258e65/include/c++/11.4.1/future:212

(etc.)

Could you please have a look?

[perrotta]

By the way, the same also happens in the normal workflows, not only in tests (still for UBSAN builds): see e.g. wfs 136.843 , 140.066 , 140.107 , 158.5

@mmusich FYI

[mmusich]

workflows, not only in tests (still for UBSAN builds): see e.g. wfs 136.843 , 140.066 , 140.107 , 158.5

Not clear what this has to do with the APE. Also from the log looks like a fwk problem to me.

[perrotta]

Yes, perhaps you are right @mmusich : I was misaddressed by seeing it first in the APE unit test added by this PR

[perrotta]

@cms-sw/core-l2 this is probably something you want to comment about

[iarspider]

@cms-sw/core-l2 this is probably something you want to comment about

#35138 , #37024 , #35036 .

[perrotta]

Thank you @iarspider
Ok, not new, nor urgent, then.

[iarspider]

Not new - yes, not urgent - I would think so, nice to fix - definitely!

[mmusich]

#35138 , #37024 , #35036 .

out of these three #37024 looks the one closer to this discussion