Fix alignment offline validation ROOT6.27/01 issue #40114

TomasKello · 2022-11-18T19:35:56Z

PR description:

Recasting pointer access that was in conflict with ROOT Version: 6.27/01 and caused unit test failure.

PR validation:

Unit tests now successful for el8_amd64_gcc10 and both ROOT Version: 6.27/01 and ROOT Version: 6.24/07.

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:

N/A

cmsbuild · 2022-11-18T19:45:05Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40114/33098

This PR adds an extra 28KB to repository

cmsbuild · 2022-11-18T19:45:24Z

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

It involves the following packages:

Alignment/OfflineValidation (alca)

@malbouis, @yuanchao, @cmsbuild, @saumyaphor4252, @francescobrivio, @ChrisMisan, @tvami can you please review it and eventually sign? Thanks.
@mmusich, @adewit, @tocheng, @tlampen 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 · 2022-11-18T20:02:40Z

@cmsbuild, please test for CMSSW_12_6_ROOT6_X

mmusich · 2022-11-18T20:17:37Z

Alignment/OfflineValidation/src/PlotAlignmentValidation.cc

+    errorflag = kTRUE;
+  }
+  TCanvas* normchi = new TCanvas((TCanvas*)fi1->Get("TrackerOfflineValidation/GlobalTrackVariables/h_normchi2"));
+  TCanvas* chiprob = new TCanvas((TCanvas*)fi1->Get("TrackerOfflineValidation/GlobalTrackVariables/h_chi2Prob"));


why do you need new here? Can't you just assign after cast?

tvami · 2022-11-18T20:33:21Z

" Unable to find CMSSW IB for CMSSW_12_6_ROOT6_X" ? that doesnt seem right, @smuzaffar ?

tvami · 2022-11-18T20:34:20Z

there is a CMSSW_12_6_ROOT6_X_2022-11-17-2300

pcanal · 2022-11-18T20:46:01Z

Alignment/OfflineValidation/src/PlotAlignmentValidation.cc

+  TCanvas* normchi = new TCanvas((TCanvas*)fi1->Get("TrackerOfflineValidation/GlobalTrackVariables/h_normchi2"));
+  TCanvas* chiprob = new TCanvas((TCanvas*)fi1->Get("TrackerOfflineValidation/GlobalTrackVariables/h_chi2Prob"));


Suggested change

TCanvas* normchi = new TCanvas((TCanvas*)fi1->Get("TrackerOfflineValidation/GlobalTrackVariables/h_normchi2"));

TCanvas* chiprob = new TCanvas((TCanvas*)fi1->Get("TrackerOfflineValidation/GlobalTrackVariables/h_chi2Prob"));

auto normchi = fi1->Get<TCanvas>("TrackerOfflineValidation/GlobalTrackVariables/h_normchi2"));

auto chiprob = fi1->Get<TCanvas>("TrackerOfflineValidation/GlobalTrackVariables/h_chi2Prob"));

Yes indeed, my initial solution was using wrong constructor. Thanks for spotting. I need to run in through the all tests again and will commit once more.

Suggested change

TCanvas* normchi = new TCanvas((TCanvas*)fi1->Get("TrackerOfflineValidation/GlobalTrackVariables/h_normchi2"));

TCanvas* chiprob = new TCanvas((TCanvas*)fi1->Get("TrackerOfflineValidation/GlobalTrackVariables/h_chi2Prob"));

auto normchi = fi1->Get<TCanvas>("TrackerOfflineValidation/GlobalTrackVariables/h_normchi2"));

normchi->GetFrame()->ResetBit(kCanDelete);

auto chiprob = fi1->Get<TCanvas>("TrackerOfflineValidation/GlobalTrackVariables/h_chi2Prob"));

chiprob->GetFrame()->ResetBit(kCanDelete);

is likely to be fixing the issue.

I take back, still crashing.

Ah great, I see your comment on bug in TPad #40091 (comment)
It works with a workaround solution. I will make a comment to remove this extra step in this code with future ROOT version.

cmsbuild · 2022-11-19T11:53:50Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40114/33101

This PR adds an extra 32KB to repository

cmsbuild · 2022-11-19T11:54:11Z

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

tvami · 2022-11-19T12:40:58Z

@cmsbuild , please test

mmusich · 2022-11-19T15:46:26Z

Unable to find CMSSW IB for CMSSW_12_6_ROOT6_X" ? that doesnt seem right

How can we test in a realistic setup ?

cmsbuild · 2022-11-19T18:04:27Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-144342/29135/summary.html
COMMIT: a4a98bd
CMSSW: CMSSW_12_6_X_2022-11-19-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/40114/29135/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: 8 differences found in the comparisons
DQMHistoTests: Total files compared: 48
DQMHistoTests: Total histograms compared: 3417239
DQMHistoTests: Total failures: 6
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3417211
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
Checked 206 log files, 48 edm output root files, 48 DQM output files
TriggerResults: no differences found

tvami · 2022-11-21T04:31:17Z

+alca

no dedicated IBs.. but the default IB tests do pass

cmsbuild · 2022-11-21T04:31:37Z

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 · 2022-11-21T11:58:04Z

+1

Fix alignment offline validation ROOT6.27/01 issue

0fc6d31

cmsbuild added this to the CMSSW_12_6_X milestone Nov 18, 2022

cmsbuild added alca-pending code-checks-pending orp-pending pending-signatures tests-pending labels Nov 18, 2022

TomasKello mentioned this pull request Nov 18, 2022

[ROOT6] Segfault in test DMRall on Alignment/OfflineValidation #40091

Closed

cmsbuild added code-checks-approved and removed code-checks-pending labels Nov 18, 2022

cmsbuild added tests-started tests-pending and removed tests-pending tests-started labels Nov 18, 2022

mmusich reviewed Nov 18, 2022

View reviewed changes

pcanal reviewed Nov 18, 2022

View reviewed changes

Apply workaround for TPad bug

a4a98bd

cmsbuild added code-checks-pending and removed code-checks-approved labels Nov 19, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Nov 19, 2022

cmsbuild removed the tests-pending label Nov 19, 2022

cmsbuild added the tests-started label Nov 19, 2022

cmsbuild added tests-approved and removed tests-started labels Nov 19, 2022

cmsbuild added alca-approved fully-signed and removed alca-pending pending-signatures labels Nov 21, 2022

cmsbuild added orp-approved and removed orp-pending labels Nov 21, 2022

cmsbuild merged commit 68eca59 into cms-sw:master Nov 21, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix alignment offline validation ROOT6.27/01 issue #40114

Fix alignment offline validation ROOT6.27/01 issue #40114

TomasKello commented Nov 18, 2022

cmsbuild commented Nov 18, 2022

cmsbuild commented Nov 18, 2022

mmusich commented Nov 18, 2022

mmusich Nov 18, 2022

tvami commented Nov 18, 2022

tvami commented Nov 18, 2022

pcanal Nov 18, 2022

TomasKello Nov 18, 2022

pcanal Nov 18, 2022

TomasKello Nov 18, 2022

TomasKello Nov 18, 2022

cmsbuild commented Nov 19, 2022

cmsbuild commented Nov 19, 2022

tvami commented Nov 19, 2022

mmusich commented Nov 19, 2022

cmsbuild commented Nov 19, 2022

tvami commented Nov 21, 2022

cmsbuild commented Nov 21, 2022

perrotta commented Nov 21, 2022

		TCanvas* normchi = new TCanvas((TCanvas*)fi1->Get("TrackerOfflineValidation/GlobalTrackVariables/h_normchi2"));
		TCanvas* chiprob = new TCanvas((TCanvas*)fi1->Get("TrackerOfflineValidation/GlobalTrackVariables/h_chi2Prob"));

-  TCanvas* normchi = new TCanvas((TCanvas*)fi1->Get("TrackerOfflineValidation/GlobalTrackVariables/h_normchi2"));
-  TCanvas* chiprob = new TCanvas((TCanvas*)fi1->Get("TrackerOfflineValidation/GlobalTrackVariables/h_chi2Prob"));
+  auto normchi = fi1->Get<TCanvas>("TrackerOfflineValidation/GlobalTrackVariables/h_normchi2"));
+  normchi->GetFrame()->ResetBit(kCanDelete);
+  auto chiprob = fi1->Get<TCanvas>("TrackerOfflineValidation/GlobalTrackVariables/h_chi2Prob"));
+  chiprob->GetFrame()->ResetBit(kCanDelete);

Fix alignment offline validation ROOT6.27/01 issue #40114

Fix alignment offline validation ROOT6.27/01 issue #40114

Conversation

TomasKello commented Nov 18, 2022

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 Nov 18, 2022

cmsbuild commented Nov 18, 2022

mmusich commented Nov 18, 2022

mmusich Nov 18, 2022

Choose a reason for hiding this comment

tvami commented Nov 18, 2022

tvami commented Nov 18, 2022

pcanal Nov 18, 2022

Choose a reason for hiding this comment

TomasKello Nov 18, 2022

Choose a reason for hiding this comment

pcanal Nov 18, 2022

Choose a reason for hiding this comment

TomasKello Nov 18, 2022

Choose a reason for hiding this comment

TomasKello Nov 18, 2022

Choose a reason for hiding this comment

cmsbuild commented Nov 19, 2022

cmsbuild commented Nov 19, 2022

tvami commented Nov 19, 2022

mmusich commented Nov 19, 2022

cmsbuild commented Nov 19, 2022

Comparison Summary

tvami commented Nov 21, 2022

cmsbuild commented Nov 21, 2022

perrotta commented Nov 21, 2022