MuonAlignment ASAN bug fix #36147

hyunyong · 2021-11-16T20:32:21Z

PR description:

Resolve #36053

The issue was the DB converter created new APEs
This PR fixed the issue; MuonAlignmentInputDB reads APEs from DB now

PR validation:

Test with Alignment/MuonAlignment/test/test_MuonGeometryDBConverter.sh

tvami · 2021-11-16T20:38:57Z

@cmsbuild , please test for CMSSW_12_2_ASAN_X

cmsbuild · 2021-11-16T20:39:06Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36147/26684

This PR adds an extra 20KB to repository

cmsbuild · 2021-11-16T20:39:20Z

A new Pull Request was created by @hyunyong for master.

It involves the following packages:

Alignment/MuonAlignment (alca)
Alignment/MuonAlignmentAlgorithms (alca)

@malbouis, @tvami, @yuanchao, @francescobrivio can you please review it and eventually sign? Thanks.
@pakhotin, @adewit, @abbiendi, @CeliaFernandez, @jhgoh, @tocheng, @mmusich, @trocino 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

tvami · 2021-11-16T20:40:19Z

type bug-fix

cmsbuild · 2021-11-17T11:33:59Z

-1

Failed Tests: UnitTests
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ca9366/20574/summary.html
COMMIT: 6b0658c
CMSSW: CMSSW_12_2_ASAN_X_2021-11-15-2300/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/36147/20574/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-ca9366/20574/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ca9366/20574/git-merge-result

Unit Tests

I found errors in the following unit tests:

---> test TestDQMServicesDemo had ERRORS

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
Reco comparison had 71 failed jobs
DQMHistoTests: Total files compared: 42
DQMHistoTests: Total histograms compared: 3320802
DQMHistoTests: Total failures: 1093685
DQMHistoTests: Total nulls: 33
DQMHistoTests: Total successes: 2227062
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: -0.17 KiB( 41 files compared)
DQMHistoSizes: changed ( 10224.0 ): 0.117 KiB SiStrip/MechanicalView
DQMHistoSizes: changed ( 136.874 ): 0.004 KiB JetMET/SUSYDQM
DQMHistoSizes: changed ( 250202.181 ): -0.533 KiB SiStrip/MechanicalView
DQMHistoSizes: changed ( 25202.0 ): 0.246 KiB SiStrip/MechanicalView
DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
Checked 177 log files, 37 edm output root files, 42 DQM output files

cmsbuild · 2021-11-23T19:40:01Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36147/26840

This PR adds an extra 12KB to repository

cmsbuild · 2021-11-23T19:40:27Z

Pull request #36147 was updated. @cmsbuild, @malbouis, @tvami, @yuanchao, @francescobrivio can you please check and sign again.

tvami · 2021-11-23T19:41:13Z

@cmsbuild , please test

cmsbuild · 2021-11-23T22:42:58Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ca9366/20720/summary.html
COMMIT: f7b1887
CMSSW: CMSSW_12_2_X_2021-11-23-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/36147/20720/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: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 42
DQMHistoTests: Total histograms compared: 3247025
DQMHistoTests: Total failures: 0
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3247003
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 41 files compared)
Checked 177 log files, 37 edm output root files, 42 DQM output files
TriggerResults: no differences found

tvami · 2021-11-24T00:09:10Z

@cmsbuild , please test for CMSSW_12_2_ASAN_X

cmsbuild · 2021-11-24T15:20:16Z

-1

Failed Tests: UnitTests
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ca9366/20724/summary.html
COMMIT: f7b1887
CMSSW: CMSSW_12_2_ASAN_X_2021-11-22-2300/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/36147/20724/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-ca9366/20724/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-ca9366/20724/git-merge-result

Unit Tests

I found errors in the following unit tests:

---> test TestDQMServicesDemo had ERRORS

Comparison Summary

Summary:

No significant changes to the logs found
ROOTFileChecks: Some differences in event products or their sizes found
Reco comparison results: 0 differences found in the comparisons
Reco comparison had 71 failed jobs
DQMHistoTests: Total files compared: 42
DQMHistoTests: Total histograms compared: 3247014
DQMHistoTests: Total failures: 1084251
DQMHistoTests: Total nulls: 28
DQMHistoTests: Total successes: 2162713
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.302 KiB( 41 files compared)
DQMHistoSizes: changed ( 10224.0 ): 0.117 KiB SiStrip/MechanicalView
DQMHistoSizes: changed ( 11634.0,... ): 0.117 KiB Physics/NanoAODDQM
DQMHistoSizes: changed ( 250202.181 ): -0.533 KiB SiStrip/MechanicalView
DQMHistoSizes: changed ( 25202.0 ): 0.246 KiB SiStrip/MechanicalView
DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
Checked 177 log files, 37 edm output root files, 42 DQM output files

tvami · 2021-11-24T15:52:41Z

Hi @cms-sw/dqm-l2 friends, @iarspider is this a known thing for the ASAN release? I don't think TestDQMServicesDemo problem is connected to this PR, can you please confirm? As you can see above it's really just coming up for the ASAN version.

iarspider · 2021-11-24T15:56:08Z

@tvami yes, it was reported it in #36063.

tvami · 2021-11-24T16:02:11Z

Thanks @iarspider !

tvami · 2021-11-24T16:02:46Z

+alca

cmsbuild · 2021-11-24T16:02:57Z

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 · 2021-11-24T16:44:22Z

This solves the bug fix reported by ASAN: good!
This does not address Deprecated legacy modules in Alignment/MuonAlignment* cms-AlCaDB/AlCaTools#43 instead: please remove it from the PR description
This further moves to the MuonAlignmentInputDB constructor with APE's, which is probably fine but it was not written in the PR description, nor a justification for it is given

@hyunyong could you please take care of adding/modifying the relevant info as above, for PR documentation? Then it can be merged.

iarspider · 2021-11-24T16:50:54Z

@hyunyong so the root issue was mismatch in MuonAlignmentInputDB / MuonAlignmentInputDB constructor arguments between declaration and usage?

hyunyong · 2021-11-24T17:13:11Z

@hyunyong so the root issue was mismatch in MuonAlignmentInputDB / MuonAlignmentInputDB constructor arguments between declaration and usage?

Yes, MuonAlignmentInputDB has to read the APEs from DB but put a wrong constructor then it creates new APEs itself.
Now MuonAlignmentInputDB reads APEs from a DB file.

perrotta · 2021-11-24T17:21:49Z

+1

(Thank you @hyunyong for the additional infos written in the PR description)

hyunyong · 2021-11-24T17:22:28Z

This solves the bug fix reported by ASAN: good!

This does not address Deprecated legacy modules in Alignment/MuonAlignment* cms-AlCaDB/AlCaTools#43 instead: please remove it from the PR description

This further moves to the MuonAlignmentInputDB constructor with APE's, which is probably fine but it was not written in the PR description, nor a justification for it is given

@hyunyong could you please take care of adding/modifying the relevant info as above, for PR documentation? Then it can be merged.

Yes, thank you for your comments.
I updated the PR description.

cmsbuild added this to the CMSSW_12_2_X milestone Nov 16, 2021

cmsbuild added alca-pending code-checks-pending orp-pending pending-signatures tests-pending labels Nov 16, 2021

cmsbuild added code-checks-approved tests-started and removed tests-pending code-checks-pending labels Nov 16, 2021

cmsbuild added the bug-fix label Nov 16, 2021

cmsbuild added tests-rejected and removed tests-started labels Nov 17, 2021

cmsbuild mentioned this pull request Nov 20, 2021

Remove deprecation warnings from Alignment subsystem #36193

Merged

rebase

f7b1887

hyunyong force-pushed the MuonAlignment_ASAN_bug_fix branch from 6b0658c to f7b1887 Compare November 23, 2021 19:31

cmsbuild added code-checks-pending tests-pending and removed tests-rejected code-checks-approved labels Nov 23, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Nov 23, 2021

cmsbuild added tests-started and removed tests-pending labels Nov 23, 2021

cmsbuild added tests-approved and removed tests-started labels Nov 23, 2021

cmsbuild added alca-approved fully-signed and removed alca-pending pending-signatures labels Nov 24, 2021

cmsbuild added orp-approved and removed orp-pending labels Nov 24, 2021

cmsbuild merged commit ad0c327 into cms-sw:master Nov 24, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MuonAlignment ASAN bug fix #36147

MuonAlignment ASAN bug fix #36147

hyunyong commented Nov 16, 2021 •

edited

Loading

tvami commented Nov 16, 2021

cmsbuild commented Nov 16, 2021

cmsbuild commented Nov 16, 2021

tvami commented Nov 16, 2021

cmsbuild commented Nov 17, 2021

cmsbuild commented Nov 23, 2021

cmsbuild commented Nov 23, 2021

tvami commented Nov 23, 2021

cmsbuild commented Nov 23, 2021

tvami commented Nov 24, 2021

cmsbuild commented Nov 24, 2021

tvami commented Nov 24, 2021

iarspider commented Nov 24, 2021

tvami commented Nov 24, 2021

tvami commented Nov 24, 2021

cmsbuild commented Nov 24, 2021

perrotta commented Nov 24, 2021

iarspider commented Nov 24, 2021

hyunyong commented Nov 24, 2021

perrotta commented Nov 24, 2021

hyunyong commented Nov 24, 2021

MuonAlignment ASAN bug fix #36147

MuonAlignment ASAN bug fix #36147

Conversation

hyunyong commented Nov 16, 2021 • edited Loading

PR description:

PR validation:

tvami commented Nov 16, 2021

cmsbuild commented Nov 16, 2021

cmsbuild commented Nov 16, 2021

tvami commented Nov 16, 2021

cmsbuild commented Nov 17, 2021

Unit Tests

Comparison Summary

cmsbuild commented Nov 23, 2021

cmsbuild commented Nov 23, 2021

tvami commented Nov 23, 2021

cmsbuild commented Nov 23, 2021

Comparison Summary

tvami commented Nov 24, 2021

cmsbuild commented Nov 24, 2021

Unit Tests

Comparison Summary

tvami commented Nov 24, 2021

iarspider commented Nov 24, 2021

tvami commented Nov 24, 2021

tvami commented Nov 24, 2021

cmsbuild commented Nov 24, 2021

perrotta commented Nov 24, 2021

iarspider commented Nov 24, 2021

hyunyong commented Nov 24, 2021

perrotta commented Nov 24, 2021

hyunyong commented Nov 24, 2021

hyunyong commented Nov 16, 2021 •

edited

Loading