New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Activate IrradiationBiasCorrection in Pixel CPE generic for Run3 #27183
Activate IrradiationBiasCorrection in Pixel CPE generic for Run3 #27183
Conversation
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-27183/10338
|
A new Pull Request was created by @mmusich (Marco Musich) for master. It involves the following packages: RecoLocalTracker/SiPixelRecHits @perrotta, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
please test |
The tests are being triggered in jenkins. |
-1 Tested at: 33eb134 You can see the results of the tests here: I found follow errors while testing this PR Failed tests: RelVals
When I ran the RelVals I found an error in the following workflows: runTheMatrix-results/20034.0_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D17_GenSimHLBeamSpotFull14+DigiFullTrigger_2023D17+RecoFullGlobal_2023D17+HARVESTFullGlobal_2023D17/step3_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D17_GenSimHLBeamSpotFull14+DigiFullTrigger_2023D17+RecoFullGlobal_2023D17+HARVESTFullGlobal_2023D17.log21234.0 step3 runTheMatrix-results/21234.0_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D21_GenSimHLBeamSpotFull14+DigiFullTrigger_2023D21+RecoFullGlobal_2023D21+HARVESTFullGlobal_2023D21/step3_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D21_GenSimHLBeamSpotFull14+DigiFullTrigger_2023D21+RecoFullGlobal_2023D21+HARVESTFullGlobal_2023D21.log27434.0 step3 runTheMatrix-results/27434.0_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D35_GenSimHLBeamSpotFull14+DigiFullTrigger_2023D35+RecoFullGlobal_2023D35+HARVESTFullGlobal_2023D35/step3_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D35_GenSimHLBeamSpotFull14+DigiFullTrigger_2023D35+RecoFullGlobal_2023D35+HARVESTFullGlobal_2023D35.log29034.0 step3 runTheMatrix-results/29034.0_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D41_GenSimHLBeamSpotFull14+DigiFullTrigger_2023D41+RecoFullGlobal_2023D41+HARVESTFullGlobal_2023D41/step3_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D41_GenSimHLBeamSpotFull14+DigiFullTrigger_2023D41+RecoFullGlobal_2023D41+HARVESTFullGlobal_2023D41.log |
Comparison not run due to runTheMatrix errors (RelVals and Igprof tests were also skipped) |
33eb134
to
e2ed5ce
Compare
Comparison is ready Comparison Summary:
|
@mmusich apart from the fix on Pixel resolution, the changes in this PR are so deep that they are affecting almost all subsystems in the Run3 wf, even HCAL and ECAL RecHits and CaloTowers... |
OK, answering myself, I realize now that ALCA conditions are different too, please ignore my previous message, sorry |
@jfernan2 the changes in the ALCA conditions are affecting only the Pixel subsystem. |
@mmusich Your candidate GTs do not contain two tags that the Hypernews thread [MC][GT][Run3] Pixel conditions for Run3 studies indicates are needed. If you confirm that these tags should be used, I can create auto:phase1_2021_realistic (106X_upgrade2021_realistic_v8) and auto:phase1_2021_cosmics (106X_upgrade2021cosmics_realistic_deco_v3) GTs for you that includes all of the 2021 pixel updates. |
@christopheralanwest it is done on purpose, please try to read carefully #27183 (comment) and #27183 (comment) ...
as far as I can see these two GTs do not exist, but if you want to make my candidates real GTs that would actually be better. You can create another set of GTs with the full package (including the other two tags) at a later time and update autoCond in another PR. |
@mmusich Thanks for pointing me to the relevant section of the thread. I would rather use the candidate GTs for now in case some other development arises and needs to be merged before this PR. Once this PR has a reco signature, I will submit a PR that cherry-picks your GT update and then updates the two GTs to use the full 2021 pixel conditions. That will allow the two PRs to be tested separately or together as needed, without generating merge conflicts. |
Here are some highlights from ttbar no-PU workflow 11624 (1K events). The track uncertainties are down, which is reflected in the PV position uncertainties (by about 20%, if I'm reading off the log scale right) About 7% fewer initialStep tracks It looks like more things than just what's immediately related to the irradiation bias correction have changed. At the pixel level (also can be seen with less stats in in the DQM gui posted earlier http://tinyurl.com/y5hjwlbw ): The x resolutions for the hits have some bias There is some small bias in FPIX x: The y pulls look better, closer to 1 (the most significant effect is in BPIX1: Gaussian width changes from 0.61 to 0.94 for a fit within -2, 2) The cluster prob changed, most visibly in BPIX1. Overall, the efficiency loss (driven by cluster prob?) seems a bit of a showstopper. I ran with Another significant effect is in FPIX, So, incrementally, just doing |
@slava77 I think we are simply correctly simulating more radiation damage now... |
+1
|
@slava77 thanks for the sign-off.
The whole CPE package (reconstruction and digitization conditions) have been updated. These come as an indivisible block as they are all derived with a target integrated dose (in this case end of 2021). |
+1 |
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. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
PR description:
In this PR the
IrradiationBiasCorrection
parameter for the Pixel CPE Generic algorithm is activated for the Run 3 era.The motivation for activating the correction is due to the expected comparatively large loss of resolution along the local y [global-z in barrel] direction for an heavily irradiated detector (as expected for the end of run3 and extended end or run3 simulation scenarios), in which carrier trapping significantly reduces Lorentz drift induced charge sharing.
As shown in this presentation, without this correction most clusters are reconstructed at ±100-200μm from true y, caused by charge loss from the backplane side.
As pointed out in the review (see #27183 (comment)) the Pixel CPE conditions need to be updated to take into account the parameter is switched on.
The
phase1_2021_realistic
andphase1_2021_cosmics
are updated accordingly.The diff of the two Global Tags is the following:
PR validation:
Code compiles and the dumping of cmsDriver output configuration confirms the desired parameter value is assigned.
The activation of the parameter has been tested (together with the changes proposed in #27181) with:
There is not yet a physics output based validation (in progress), We will update the description once comparisons are available.Physics validation has been shown in this presentation at the RECO / AT meeting of 14 Jun 2019 link.
if this PR is a backport please specify the original PR:
This PR is not a backport
cc: @tsusa @tvami @leaca @pmaksim1 @cmantill @OzAmram