Run3-alca249 Modify calibration macros to use the current IsoTrack calibration #46410

bsunanda · 2024-10-16T14:24:10Z

PR description:

Modify calibration macros to use the current IsoTrack calibration. We introduce a new mode (truncate flag 9) in place of its earlier implementation. We have a new depth-dependent IsoTrack calibration where parameters for depth = 1 and 2 are kept the same

PR validation:

Use the macros for the current IsoTrack analysis

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:

Nothing special

cmsbuild · 2024-10-16T14:24:31Z

cms-bot internal usage

cmsbuild · 2024-10-16T14:25:31Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46410/42258

cmsbuild · 2024-10-16T14:25:54Z

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

It involves the following packages:

Calibration/HcalCalibAlgos (alca)

@atpathak, @cmsbuild, @consuegs, @perrotta can you please review it and eventually sign? Thanks.
@mmusich, @rsreds, @tocheng, @yuanchao this is something you requested to watch as well.
@antoniovilela, @mandrenguyen, @rappoccio, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

cmsbuild · 2024-10-16T14:33:30Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46410/42259

Code check has found code style and quality issues which could be resolved by applying following patch(s)

code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46410/42259/code-format.patch
e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46410/42259/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

cmsbuild · 2024-10-16T14:38:39Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46410/42260

cmsbuild · 2024-10-16T14:39:05Z

Pull request #46410 was updated. @atpathak, @cmsbuild, @consuegs, @perrotta can you please check and sign again.

bsunanda · 2024-10-16T14:40:15Z

@cmsbuild Please test

cmsbuild · 2024-10-16T17:49:02Z

-1

Failed Tests: RelVals-INPUT
Size: This PR adds an extra 96KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-916676/42242/summary.html
COMMIT: 88fab0d
CMSSW: CMSSW_14_2_X_2024-10-15-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/46410/42242/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals-INPUT

2022.1010012022.101001_RunEGamma2022C_10k/step1_dasquery.log
2023.0000012023.000001_RunMuonEG2023C_10k/step1_dasquery.log
2023.0010012023.001001_RunDisplacedJet2023C_10k/step1_dasquery.log

Expand to see more relval errors ...

2023.100001
2022.101001
2023.000001
2023.001001
2023.100001

Comparison Summary

Summary:

You potentially removed 6 lines from the logs
Reco comparison results: 2 differences found in the comparisons
DQMHistoTests: Total files compared: 44
DQMHistoTests: Total histograms compared: 3335129
DQMHistoTests: Total failures: 455
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3334654
DQMHistoTests: Total skipped: 20
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 43 files compared)
Checked 193 log files, 163 edm output root files, 44 DQM output files
TriggerResults: no differences found

perrotta · 2024-10-17T10:41:41Z

Calibration/HcalCalibAlgos/macros/CalibCorr.C

-      else
-        depth = 1;
-    }
+    //Depths 1 and 2 considered as depth = 1; rest of the depths are kept as it is


The comment does not correspond to what is implemented in the code (it should be

Suggested change

//Depths 1 and 2 considered as depth = 1; rest of the depths are kept as it is

//Depths 1 and 2 considered as depth = 2; rest of the depths are kept as it is

instead)

And in any case the implementation here is quite different from what was before: please check and explain what was updated in HB and HE that requires this change

Like to have the same constants for depths 1 and 2 otherwise depth-dependent constants. The comment is changed as suggested

cmsbuild · 2024-10-17T11:09:16Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46410/42278

cmsbuild · 2024-10-17T11:09:38Z

Pull request #46410 was updated. @atpathak, @cmsbuild, @consuegs, @perrotta can you please check and sign again.

bsunanda · 2024-10-17T11:13:03Z

@cmsbuild Please test

perrotta · 2024-10-17T11:54:31Z

@bsunanda just to be clear, this PR changes the implementation as follows

=== HB moved from

depth_current depth_withPR
     1            1
     2            2
     3            2
     4            2

to

depth_current depth_withPR
     1            2
     2            2
     3            3
     4            4

=== HE moved from

depth_current depth_withPR
     1            1
     2            2
     3            3
     4            4

to

depth_current depth_withPR
     1            2
     2            2
     3            3
     4            4

I assume that this is what you want, but since it is not explained anywhere in the PR description please confirm (even better if you can link where in the "current IsoTrack calibration" this is implemented).

bsunanda · 2024-10-17T12:38:12Z

@perrotta I tried to explain in the description now

cmsbuild · 2024-10-17T15:04:58Z

+1

Size: This PR adds an extra 96KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-916676/42268/summary.html
COMMIT: 67a9557
CMSSW: CMSSW_14_2_X_2024-10-16-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/46410/42268/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: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 44
DQMHistoTests: Total histograms compared: 3335129
DQMHistoTests: Total failures: 458
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3334651
DQMHistoTests: Total skipped: 20
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 43 files compared)
Checked 193 log files, 163 edm output root files, 44 DQM output files
TriggerResults: no differences found

perrotta · 2024-10-17T15:37:18Z

+alca

cmsbuild · 2024-10-17T15:37:40Z

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

mandrenguyen · 2024-10-18T05:42:13Z

+1

Modify calibration macros to use current IsoTrack calibration

de4b21a

cmsbuild added this to the CMSSW_14_2_X milestone Oct 16, 2024

cmsbuild added alca-pending pending-signatures tests-pending orp-pending code-checks-pending labels Oct 16, 2024

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 16, 2024

Proper changes

90aa3da

cmsbuild added code-checks-pending and removed code-checks-approved labels Oct 16, 2024

cmsbuild added code-checks-rejected and removed code-checks-pending labels Oct 16, 2024

Code check

88fab0d

cmsbuild added code-checks-pending and removed code-checks-rejected labels Oct 16, 2024

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 16, 2024

cmsbuild added tests-started and removed tests-pending labels Oct 16, 2024

cmsbuild removed the tests-started label Oct 16, 2024

cmsbuild added the tests-started label Oct 17, 2024

perrotta reviewed Oct 17, 2024

View reviewed changes

Update the comment in CalibCorr.C

67a9557

cmsbuild added tests-pending code-checks-pending and removed tests-started code-checks-approved labels Oct 17, 2024

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 17, 2024

cmsbuild added tests-started and removed tests-pending labels Oct 17, 2024

cmsbuild added tests-approved and removed tests-started labels Oct 17, 2024

cmsbuild added alca-approved fully-signed and removed alca-pending pending-signatures labels Oct 17, 2024

cmsbuild added orp-approved and removed orp-pending labels Oct 18, 2024

cmsbuild merged commit 6c9b76b into cms-sw:master Oct 18, 2024
11 checks passed

cmsbuild mentioned this pull request Oct 19, 2024

[vdt]Apply patch to avoid UBSan signed integer overflow error cms-sw/cmsdist#9470

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Run3-alca249 Modify calibration macros to use the current IsoTrack calibration #46410

Run3-alca249 Modify calibration macros to use the current IsoTrack calibration #46410

bsunanda commented Oct 16, 2024 •

edited

Loading

cmsbuild commented Oct 16, 2024 •

edited

Loading

cmsbuild commented Oct 16, 2024

cmsbuild commented Oct 16, 2024

cmsbuild commented Oct 16, 2024

cmsbuild commented Oct 16, 2024

cmsbuild commented Oct 16, 2024

bsunanda commented Oct 16, 2024

cmsbuild commented Oct 16, 2024

perrotta Oct 17, 2024

bsunanda Oct 17, 2024

cmsbuild commented Oct 17, 2024

cmsbuild commented Oct 17, 2024

bsunanda commented Oct 17, 2024

perrotta commented Oct 17, 2024

bsunanda commented Oct 17, 2024

cmsbuild commented Oct 17, 2024

perrotta commented Oct 17, 2024

cmsbuild commented Oct 17, 2024

mandrenguyen commented Oct 18, 2024

	//Depths 1 and 2 considered as depth = 1; rest of the depths are kept as it is
	//Depths 1 and 2 considered as depth = 2; rest of the depths are kept as it is

Run3-alca249 Modify calibration macros to use the current IsoTrack calibration #46410

Run3-alca249 Modify calibration macros to use the current IsoTrack calibration #46410

Conversation

bsunanda commented Oct 16, 2024 • edited Loading

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 Oct 16, 2024 • edited Loading

cmsbuild commented Oct 16, 2024

cmsbuild commented Oct 16, 2024

cmsbuild commented Oct 16, 2024

cmsbuild commented Oct 16, 2024

cmsbuild commented Oct 16, 2024

bsunanda commented Oct 16, 2024

cmsbuild commented Oct 16, 2024

RelVals-INPUT

Comparison Summary

perrotta Oct 17, 2024

Choose a reason for hiding this comment

bsunanda Oct 17, 2024

Choose a reason for hiding this comment

cmsbuild commented Oct 17, 2024

cmsbuild commented Oct 17, 2024

bsunanda commented Oct 17, 2024

perrotta commented Oct 17, 2024

bsunanda commented Oct 17, 2024

cmsbuild commented Oct 17, 2024

Comparison Summary

perrotta commented Oct 17, 2024

cmsbuild commented Oct 17, 2024

mandrenguyen commented Oct 18, 2024

bsunanda commented Oct 16, 2024 •

edited

Loading

cmsbuild commented Oct 16, 2024 •

edited

Loading