Use anonymous namespace in CandIsolatorFromDeposits #40984

Dr15Jones · 2023-03-07T17:37:43Z

PR description:

This appears to avoid an ASAN error when used with LTO.

PR validation:

Building and running in the ASAN IB now allows the 1.0 step3 workflow to run.

This appears to avoid an ASAN error when used with LTO.

cmsbuild · 2023-03-07T17:45:20Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40984/34491

This PR adds an extra 12KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File PhysicsTools/IsolationAlgos/plugins/CandIsolatorFromDeposits.cc modified in PR(s): Use std::from_chars instead of std::regex+std::atof #40956, use std::stof with try/catch instead of std::regex #40915

cmsbuild · 2023-03-07T17:45:47Z

A new Pull Request was created by @Dr15Jones (Chris Jones) for master.

It involves the following packages:

PhysicsTools/IsolationAlgos (reconstruction)

@cmsbuild, @mandrenguyen, @clacaputo can you please review it and eventually sign? Thanks.
@perrotta, @dpiparo, @rappoccio you are the release manager for this.

cms-bot commands are listed here

Dr15Jones · 2023-03-07T17:49:10Z

please test

dan131riley · 2023-03-07T18:09:06Z

I tested with step1 of wf 5.1, and it worked there too. Do not understand at all why it fixes it.

cmsbuild · 2023-03-07T22:20:44Z

-1

Failed Tests: UnitTests
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-65f126/31127/summary.html
COMMIT: 1b2a30d
CMSSW: CMSSW_13_1_X_2023-03-07-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/40984/31127/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-65f126/31127/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-65f126/31127/git-merge-result

Unit Tests

I found errors in the following unit tests:

---> test test-das-selected-lumis had ERRORS

Comparison Summary

Summary:

You potentially removed 9 lines from the logs
ROOTFileChecks: Some differences in event products or their sizes found
Reco comparison results: 10 differences found in the comparisons
DQMHistoTests: Total files compared: 49
DQMHistoTests: Total histograms compared: 3529718
DQMHistoTests: Total failures: 51
DQMHistoTests: Total nulls: 12
DQMHistoTests: Total successes: 3529633
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 377.512 KiB( 48 files compared)
DQMHistoSizes: changed ( 11634.0,... ): 61.269 KiB HLT/HLTEgammaValidation
DQMHistoSizes: changed ( 11634.0,... ): 1.414 KiB HLT/Filters
Checked 213 log files, 164 edm output root files, 49 DQM output files
TriggerResults: found differences in 6 / 47 workflows

Dr15Jones · 2023-03-07T22:26:41Z

The failing test had the message

2023/03/07 19:34:19 ERROR failed to parse X509 proxy: crypto/tls: failed to parse key: asn1: syntax error: data truncated

which could not have been from this change.

Dr15Jones · 2023-03-07T22:37:54Z

please test

let's see if this can just clear the spurious failure.

makortel · 2023-03-07T22:39:54Z

@cmsbuild, please test for CMSSW_13_1_ASAN_X

let's see the ASAN as well

cmsbuild · 2023-03-08T01:54:23Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-65f126/31135/summary.html
COMMIT: 1b2a30d
CMSSW: CMSSW_13_1_X_2023-03-07-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/40984/31135/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:

@smuzaffar [DB] Various fixes/improvements for unit test #40975
@smuzaffar [SIMULATION] Various fixes/improvements for unit test #40974
@cms-tsg-storm HLT menu development for 13_0_X (3/N) [13_1_X] #40965
@smuzaffar [ALCA] Various fixes/improvements for unit test #40972
@mmusich SiPixelDynamicInefficiency_PayloadInspector fix issue with PU parametrization plot polynomial representation #40981
@bsunanda Phase2-hgx336A Correct the nonzzero cassette shift scenario for HGCal #40959
@smuzaffar [ALCA-DB] Various fixes/improvements for unit test #40978
@bsunanda Run3-alca237X Add the possibility of providing depth-dependent correction factors from muon studies to IsoTrack analysis in HCAL calibration #40968

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

Comparison Summary

Summary:

No significant changes to the logs found
ROOTFileChecks: Some differences in event products or their sizes found
Reco comparison results: 10 differences found in the comparisons
DQMHistoTests: Total files compared: 49
DQMHistoTests: Total histograms compared: 3529718
DQMHistoTests: Total failures: 53
DQMHistoTests: Total nulls: 12
DQMHistoTests: Total successes: 3529631
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 377.512 KiB( 48 files compared)
DQMHistoSizes: changed ( 11634.0,... ): 61.269 KiB HLT/HLTEgammaValidation
DQMHistoSizes: changed ( 11634.0,... ): 1.414 KiB HLT/Filters
Checked 213 log files, 164 edm output root files, 49 DQM output files
TriggerResults: found differences in 6 / 47 workflows

cmsbuild · 2023-03-08T02:35:41Z

-1

Failed Tests: UnitTests
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-65f126/31136/summary.html
COMMIT: 1b2a30d
CMSSW: CMSSW_13_1_ASAN_X_2023-03-06-2300/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/40984/31136/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-65f126/31136/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-65f126/31136/git-merge-result

Unit Tests

I found errors in the following unit tests:

---> test GCPall had ERRORS
---> test SubmitPVrbr had ERRORS
---> test SubmitPVsplit had ERRORS
---> test JetHTall had ERRORS
and more ...

smuzaffar · 2023-03-08T10:50:56Z

@cms-sw/reconstruction-l2 can you please review this? This should fix the massive ASAN relval errors (please ignore the ASAN unit tests errors #40984 (comment))

clacaputo · 2023-03-08T11:01:41Z

+reconstruction

technical PR for ASAN build
reco differences are related to HLT menu development for 13_0_X (3/N) [13_1_X] #40965

cmsbuild · 2023-03-08T11:02:02Z

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-08T11:04:36Z

+1

smuzaffar · 2023-03-08T11:10:47Z

thanks a lot for quick action, I have triggered a 13h00 ASAN IB to test this change

Use anonymous namespace in CandIsolatorFromDeposits

1b2a30d

This appears to avoid an ASAN error when used with LTO.

cmsbuild added this to the CMSSW_13_1_X milestone Mar 7, 2023

cmsbuild added reconstruction-pending pending-signatures tests-pending orp-pending code-checks-pending labels Mar 7, 2023

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

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

This was referenced Mar 7, 2023

Maintenance tasks of week 10 cms-sw/framework-team#510

Closed

Use std::from_chars instead of std::regex+std::atof #40956

Closed

[ASAN] global-buffer-overflow in std::regex construction #40902

Closed

cmsbuild added tests-rejected and removed tests-started labels Mar 7, 2023

cmsbuild added tests-started and removed tests-rejected labels Mar 7, 2023

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

smuzaffar mentioned this pull request Mar 8, 2023

use std::stof with try/catch instead of std::regex #40915

Closed

cmsbuild added reconstruction-approved fully-signed and removed reconstruction-pending pending-signatures labels Mar 8, 2023

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

cmsbuild merged commit d1db54c into cms-sw:master Mar 8, 2023

Dr15Jones deleted the namespaceCandIsolatorFromDeposits branch March 17, 2023 14:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use anonymous namespace in CandIsolatorFromDeposits #40984

Use anonymous namespace in CandIsolatorFromDeposits #40984

Dr15Jones commented Mar 7, 2023

cmsbuild commented Mar 7, 2023

cmsbuild commented Mar 7, 2023

Dr15Jones commented Mar 7, 2023

dan131riley commented Mar 7, 2023

cmsbuild commented Mar 7, 2023

Dr15Jones commented Mar 7, 2023

Dr15Jones commented Mar 7, 2023

makortel commented Mar 7, 2023

cmsbuild commented Mar 8, 2023

cmsbuild commented Mar 8, 2023

smuzaffar commented Mar 8, 2023

clacaputo commented Mar 8, 2023

cmsbuild commented Mar 8, 2023

perrotta commented Mar 8, 2023

smuzaffar commented Mar 8, 2023

Use anonymous namespace in CandIsolatorFromDeposits #40984

Use anonymous namespace in CandIsolatorFromDeposits #40984

Conversation

Dr15Jones commented Mar 7, 2023

PR description:

PR validation:

cmsbuild commented Mar 7, 2023

cmsbuild commented Mar 7, 2023

Dr15Jones commented Mar 7, 2023

dan131riley commented Mar 7, 2023

cmsbuild commented Mar 7, 2023

Unit Tests

Comparison Summary

Dr15Jones commented Mar 7, 2023

Dr15Jones commented Mar 7, 2023

makortel commented Mar 7, 2023

cmsbuild commented Mar 8, 2023

Comparison Summary

cmsbuild commented Mar 8, 2023

Unit Tests

smuzaffar commented Mar 8, 2023

clacaputo commented Mar 8, 2023

cmsbuild commented Mar 8, 2023

perrotta commented Mar 8, 2023

smuzaffar commented Mar 8, 2023