Update Pixel gain calibration scheme (for Run3) #492
Conversation
Validation summaryReference release CMSSW_11_1_0_pre8 at 442ae07 Validation plots/RelValTTbar_14TeV/CMSSW_11_0_0_patch1-PU_110X_mcRun3_2021_realistic_v6-v1/GEN-SIM-DIGI-RAW
/RelValZMM_14/CMSSW_11_0_0_patch1-110X_mcRun3_2021_realistic_v6-v1/GEN-SIM-DIGI-RAW
/RelValZEE_14/CMSSW_11_0_0_patch1-110X_mcRun3_2021_realistic_v6-v1/GEN-SIM-DIGI-RAW
Throughput plots/EphemeralHLTPhysics1/Run2018D-v1/RAW run=323775 lumi=53
logs and
|
Can you elaborate what do you mean by this? |
I mean if DPG (or whoever takes over the maintenance) is fine with the hardcoded approach, I don't object. |
The DPG (at least me) would prefer the python configurable approach, but since Vincenzo is a strong supporter of the hardcoded approach, which I believe has its advantages, I am not going to argue with it. In any case we don't expect to change these numbers again in Run-3. |
|
Ops, when I ran the validation I forgot to actually upload the results - they should be there now. I don't think we are able to spot the change with our validation, though ? |
|
The differences should be between "black" (CMSSW_11_1_X_Patatrack) and "orange" (this PR), and I don't see the black points anywhere, so I'm assuming they are exactly beneath the orange ones ? |
|
This is my impression as well: black and orange identical to essentially last bit |
|
Yes... so now I'm wondering if that is expected, or if for some reason the changes are not being exercised. |
|
The latter:
The workflows are still the ones from 2018, even though the samples are from 2021 MC ! Will fix... |
|
and that what you would expect for |
|
Yes, I remembered... the problem is that the |
Validation summaryReference release CMSSW_11_1_0_pre8 at 6f854b2288db Validation plots/RelValTTbar_14TeV/CMSSW_11_0_0_patch1-PU_110X_mcRun3_2021_realistic_v6-v1/GEN-SIM-DIGI-RAW
/RelValZMM_14/CMSSW_11_0_0_patch1-110X_mcRun3_2021_realistic_v6-v1/GEN-SIM-DIGI-RAW
/RelValZEE_14/CMSSW_11_0_0_patch1-110X_mcRun3_2021_realistic_v6-v1/GEN-SIM-DIGI-RAW
Throughput plots/EphemeralHLTPhysics1/Run2018D-v1/RAW run=323775 lumi=53
logs and
|
|
Mhm, that didn't really work very well :-( |
|
For my own education, what's the plan to integrate these changes? |
|
The plan is
|
|
After running more checks both online and offline, my conclusion is that the results - and thus this change - are tied to the conditions being used:
@mmusich way the change does not trigger any errors ? is it because the parameters were chaged in the same type of payload ? That seems like the "quick and dirty" solution, that is easier for whoever made the changes, and potentially causing problems everywhere else... What is the first global tag(s) that use the new conditions ? |
This is pretty obvious to understand when looking to the PR in which the change was proposed (that I pointed to in the discussion at cms-sw#27983 (comment)).
Why should it trigger any errors? The payloads are totally valid, just the numbers in them have a different interpretation starting from a given pre-release (11_1_0_pre7 to be precise). The VCal calibration is included in the DB rather than in the code.
I don't see any other way of doing it. We adjusted the code everywhere relevant and has also been spotted by us here....
Only Run3 MC Global Tags are changed. The first ones containing the new payloads are the ones at: cms-sw#29333 |
Yes, I found out that it is
Because the meaning of the numbers changed.
Making a different payload type would have ensured that it would be automatically spotted in any affected workflow. Instead, with this approach there is nothing that prevents people from mixing old code and new conditions (or vice versa), silently leading to wrong results. |
We have considered this option but we did not find it worth it.
Well, a part that this is common practice in many areas, it's extremely easy to make it correct. Run3 uses the new convention, Run2 uses the old one. So the only way of mixing wrong code with conditions is to use Run2 code with Run3 GTs or viceversa and I think that you can agree with me that if one does so, one accepts the risks of it... |
... or obsolete Run 3 tags. But thanks for spotting it also here, so we could fix it. |
Validation summaryReference release CMSSW_11_1_0_pre8 at b93ca7fd7943 Validation plots/RelValTTbar_14TeV/CMSSW_11_0_0_patch1-PU_110X_mcRun3_2021_realistic_v6-v1/GEN-SIM-DIGI-RAW
/RelValZMM_14/CMSSW_11_0_0_patch1-110X_mcRun3_2021_realistic_v6-v1/GEN-SIM-DIGI-RAW
/RelValZEE_14/CMSSW_11_0_0_patch1-110X_mcRun3_2021_realistic_v6-v1/GEN-SIM-DIGI-RAW
Throughput plots/EphemeralHLTPhysics1/Run2018D-v1/RAW run=323775 lumi=53
logs and
|
|
Looks OK to me... @VinInn could you double check the validation plots ? |
|
5 and 501 are identical to last bit (expected) |
|
OK... I think the best comparison I can do is of "orange" (testing) from the last validation, using this PR and the updated conditions: vs "black" (development) from the previous one, that used the original conditions: which look identical by eye.
|
fwyzard
changed the title
Update the Patatrack code following cms-sw#29333: Modify the scheme of the pixel gain calibration: instead of applying the VCal calibration in the clusterizer include it already in the gain calibration payload.
Update the Patatrack code following cms-sw#29333: Modify the scheme of the pixel gain calibration: instead of applying the VCal calibration in the clusterizer include it already in the gain calibration payload.
Update the Patatrack code following cms-sw#29333: Modify the scheme of the pixel gain calibration: instead of applying the VCal calibration in the clusterizer include it already in the gain calibration payload.
Update the Patatrack code following cms-sw#29333: Modify the scheme of the pixel gain calibration: instead of applying the VCal calibration in the clusterizer include it already in the gain calibration payload.
Update the Patatrack code following cms-sw#29333: Modify the scheme of the pixel gain calibration: instead of applying the VCal calibration in the clusterizer include it already in the gain calibration payload.
Update the Patatrack code following cms-sw#29333: Modify the scheme of the pixel gain calibration: instead of applying the VCal calibration in the clusterizer include it already in the gain calibration payload.
Update the Patatrack code following cms-sw#29333: Modify the scheme of the pixel gain calibration: instead of applying the VCal calibration in the clusterizer include it already in the gain calibration payload.
Update the Patatrack code following cms-sw#29333: Modify the scheme of the pixel gain calibration: instead of applying the VCal calibration in the clusterizer include it already in the gain calibration payload.
Update the Patatrack code following cms-sw#29333: Modify the scheme of the pixel gain calibration: instead of applying the VCal calibration in the clusterizer include it already in the gain calibration payload.
Update the Patatrack code following cms-sw#29333: Modify the scheme of the pixel gain calibration: instead of applying the VCal calibration in the clusterizer include it already in the gain calibration payload.
Update the Patatrack code following cms-sw#29333: Modify the scheme of the pixel gain calibration: instead of applying the VCal calibration in the clusterizer include it already in the gain calibration payload.
Update the Patatrack code following cms-sw#29333: Modify the scheme of the pixel gain calibration: instead of applying the VCal calibration in the clusterizer include it already in the gain calibration payload.
as title says...
references
cms-sw#27983 (comment)
https://github.com/cms-sw/cmssw/blob/master/RecoLocalTracker/SiPixelClusterizer/python/SiPixelClusterizer_cfi.py#L7-L28