Skip to content
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

Production of MC samples for pixel efficiency studies at High PU #1

Closed
5 tasks done
sroychow opened this issue Jan 23, 2023 · 80 comments
Closed
5 tasks done

Production of MC samples for pixel efficiency studies at High PU #1

sroychow opened this issue Jan 23, 2023 · 80 comments

Comments

@sroychow
Copy link
Contributor

sroychow commented Jan 23, 2023

To study pixel efficiency at high pu, Zmumu samples are needed. The idea is the following:-

  • Create payloads with different double column efficiencies (Marton) SiPixelDynamicInefficiency
  • Upload and request GTs on top of 124X_mcRun3_2022_realistic_postEE_Queue . The idea is to re-use GEN-SIM from 124X production and re-run the DIGI with PU mixing & RECO steps. @ Sanjana
  • Test driver commands locally @sroychow
  • Request sample to AlCA @sroychow
  • Efficiency studies (@marton @Tiziano)
@sroychow
Copy link
Contributor Author

Files at

 /afs/cern.ch/user/m/mbartok/public/pixel_dynamic_inefficiency_payloads/2023/dcol_scan_L1_L2/SiPixelDynamicInefficiency_L1_col100.db
/afs/cern.ch/user/m/mbartok/public/pixel_dynamic_inefficiency_payloads/2023/dcol_scan_L1_L2/SiPixelDynamicInefficiency_L1_col80.db
/afs/cern.ch/user/m/mbartok/public/pixel_dynamic_inefficiency_payloads/2023/dcol_scan_L1_L2/SiPixelDynamicInefficiency_L1_col85.db
/afs/cern.ch/user/m/mbartok/public/pixel_dynamic_inefficiency_payloads/2023/dcol_scan_L1_L2/SiPixelDynamicInefficiency_L1_col90.db
/afs/cern.ch/user/m/mbartok/public/pixel_dynamic_inefficiency_payloads/2023/dcol_scan_L1_L2/SiPixelDynamicInefficiency_L1_col95.db
/afs/cern.ch/user/m/mbartok/public/pixel_dynamic_inefficiency_payloads/2023/dcol_scan_L1_L2/SiPixelDynamicInefficiency_L2_col100.db
/afs/cern.ch/user/m/mbartok/public/pixel_dynamic_inefficiency_payloads/2023/dcol_scan_L1_L2/SiPixelDynamicInefficiency_L2_col80.db
/afs/cern.ch/user/m/mbartok/public/pixel_dynamic_inefficiency_payloads/2023/dcol_scan_L1_L2/SiPixelDynamicInefficiency_L2_col85.db
/afs/cern.ch/user/m/mbartok/public/pixel_dynamic_inefficiency_payloads/2023/dcol_scan_L1_L2/SiPixelDynamicInefficiency_L2_col90.db
/afs/cern.ch/user/m/mbartok/public/pixel_dynamic_inefficiency_payloads/2023/dcol_scan_L1_L2/SiPixelDynamicInefficiency_L2_col95.db

@mmusich
Copy link

mmusich commented Jan 24, 2023

For the record here are plots obtained with the following

bash script
#!/bin/bash
# Save current working dir so img can be outputted there later
W_DIR=$(pwd);
source /afs/cern.ch/cms/cmsset_default.sh;
eval `scram run -sh`;
# Go back to original working directory
cd $W_DIR;

if [ -d $W_DIR/plots_DynIneff ]; then
    rm -fr $W_DIR/plots_DynIneff
fi

mkdir -p $W_DIR/plots_DynIneff

tags=(SiPixelDynamicInefficiency_L1_col100 SiPixelDynamicInefficiency_L1_col80 SiPixelDynamicInefficiency_L1_col85 SiPixelDynamicInefficiency_L1_col90 SiPixelDynamicInefficiency_L1_col95 SiPixelDynamicIn
efficiency_L2_col100 SiPixelDynamicInefficiency_L2_col80 SiPixelDynamicInefficiency_L2_col85 SiPixelDynamicInefficiency_L2_col90 SiPixelDynamicInefficiency_L2_col95)
plots=(Geom ColGeom ChipGeom)

for i in "${tags[@]}" 
do
    for j in "${plots[@]}" 
    do
	getPayloadData.py \
	        --plugin pluginSiPixelDynamicInefficiency_PayloadInspector \
	        --plot plot_SiPixelDynamicInefficiency${j}FactorMap \
	        --tag ${i} \
	        --time_type Run \
	        --iovs '{"start_iov": "1", "end_iov": "1"}' \
	        --db sqlite_file:/afs/cern.ch/user/m/mbartok/public/pixel_dynamic_inefficiency_payloads/2023/dcol_scan_L1_L2/${i}.db \
	        --test;

	mv *.png $W_DIR/plots_DynIneff/${j}_${i}.png
    done

    getPayloadData.py \
	--plugin pluginSiPixelDynamicInefficiency_PayloadInspector \
	--plot plot_SiPixelDynamicInefficiencyPUPixelMaps \
	--tag ${i} \
	--time_type Run \
	--iovs '{"start_iov": "1", "end_iov": "1"}' \
	--db sqlite_file:/afs/cern.ch/user/m/mbartok/public/pixel_dynamic_inefficiency_payloads/2023/dcol_scan_L1_L2/${i}.db \
	--test;
    
    mv *.png $W_DIR/plots_DynIneff/PU_${i}.png
done

that validate the internal content of the payloads posted at #1 (comment)

@sroychow
Copy link
Contributor Author

@sroychow
Copy link
Contributor Author

Sample production request:-
https://its.cern.ch/jira/browse/PDMVMCPROD-78

@sroychow
Copy link
Contributor Author

sroychow commented Jan 30, 2023

@ferencek
Copy link
Collaborator

ferencek commented Feb 7, 2023

The location of the PixelHistoMaker output files on the CERN EOS:

Data (runs 362433 to 362657 from /Muon/Run2022G-SiPixelCalSingleMuonTight-PromptReco-v1/ALCARECO):
/eos/user/t/tbevilac/SWAN_projects/EPR/inputs/merged_runs362433To362657.root

MC:

/eos/cms/store/group/dpg_tracker_pixel/comm_pixel/PU_MCs/PU_MC_L1_100/
/eos/cms/store/group/dpg_tracker_pixel/comm_pixel/PU_MCs/PU_MC_L1_95/
/eos/cms/store/group/dpg_tracker_pixel/comm_pixel/PU_MCs/PU_MC_L1_90/
/eos/cms/store/group/dpg_tracker_pixel/comm_pixel/PU_MCs/PU_MC_L1_85/
/eos/cms/store/group/dpg_tracker_pixel/comm_pixel/PU_MCs/PU_MC_L1_80/

The above files are being copied to the CERN EOS.

@sroychow
Copy link
Contributor Author

First pass of results

@ferencek
Copy link
Collaborator

ferencek commented Feb 15, 2023

I requested transfer of the produced RelVals to T2_HU_Budapest to protect them from being automatically deleted based on the standard 3-month lifetime for RelVals. Below is the list of Rucio rule IDs:

Rucio Rule ID                       Dataset
21411354388e4456a74e3098421f05bb    cms:/RelValZMM_PU_13p6/CMSSW_12_4_11_patch3-PU_124X_mcRun3_2022_realistic_postEE_forPixelIneff_L2_col100_v1_MCProd78_v2-v1/GEN-SIM-RECO
286b02e2dcba4d8ab362268ef1e18614    cms:/RelValZMM_PU_13p6/CMSSW_12_4_11_patch3-PU_124X_mcRun3_2022_realistic_postEE_forPixelIneff_L2_col95_v1_MCProd78_v2-v1/GEN-SIM-RECO
569a7d2824214592822cab42f6d8c20a    cms:/RelValZMM_PU_13p6/CMSSW_12_4_11_patch3-PU_124X_mcRun3_2022_realistic_postEE_forPixelIneff_L2_col85_v1_MCProd78_v2-v1/GEN-SIM-RECO
5a42c4b47c1d4ff0a325ba745730a761    cms:/RelValZMM_PU_13p6/CMSSW_12_4_11_patch3-PU_124X_mcRun3_2022_realistic_postEE_v1_MCProd78_v2-v1/GEN-SIM-RECO
61d899fe03ba47f785d64e352fd9d928    cms:/RelValZMM_PU_13p6/CMSSW_12_4_11_patch3-PU_124X_mcRun3_2022_realistic_postEE_forPixelIneff_L2_col80_v1_MCProd78_v2-v1/GEN-SIM-RECO
88bac8449646444693f460ee2a858865    cms:/RelValZMM_PU_13p6/CMSSW_12_4_11_patch3-PU_124X_mcRun3_2022_realistic_postEE_forPixelIneff_L2_col90_v1_MCProd78_v2-v1/GEN-SIM-RECO
debe319288a64dcb963d75c2532c0a01    cms:/RelValZMM_PU_13p6/CMSSW_12_4_11_patch3-PU_124X_mcRun3_2022_realistic_postEE_forPixelIneff_L1_col85_v1_MCProd78_v2-v1/GEN-SIM-RECO
e08e24659f4c451c8bd8f539bdce4362    cms:/RelValZMM_PU_13p6/CMSSW_12_4_11_patch3-PU_124X_mcRun3_2022_realistic_postEE_forPixelIneff_L1_col100_v1_MCProd78_v2-v1/GEN-SIM-RECO
ef4f978c2cb94b27a7d4d48980f646e6    cms:/RelValZMM_PU_13p6/CMSSW_12_4_11_patch3-PU_124X_mcRun3_2022_realistic_postEE_forPixelIneff_L1_col80_v1_MCProd78_v2-v1/GEN-SIM-RECO
f6d5f9b40c994494a008a826d5eb834b    cms:/RelValZMM_PU_13p6/CMSSW_12_4_11_patch3-PU_124X_mcRun3_2022_realistic_postEE_forPixelIneff_L1_col90_v1_MCProd78_v2-v1/GEN-SIM-RECO
fd0ccdbf5b964888889b516af05e92c3    cms:/RelValZMM_PU_13p6/CMSSW_12_4_11_patch3-PU_124X_mcRun3_2022_realistic_postEE_forPixelIneff_L1_col95_v1_MCProd78_v2-v1/GEN-SIM-RECO

@mroguljic
Copy link
Collaborator

@mmusich
Copy link

mmusich commented Feb 16, 2023

For the record PI plot of the PU factors for the new tag:
image

@sroychow
Copy link
Contributor Author

Relvals requested here:-

https://its.cern.ch/jira/browse/PDMVRELVALS-188

@mmusich
Copy link

mmusich commented Feb 21, 2023

I had a very quick look to the RelValTTbar_SemiLeptonic sample requested in https://its.cern.ch/jira/browse/PDMVRELVALS-188 and honestly I don't understand what's going on

Screenshot from 2023-02-21 13-03-13

@bartokm
Copy link

bartokm commented Feb 21, 2023

I wanted to check, what to expect from the previous version of the .db and I've realized the following mistake... (In short: the dyn.ineff. factors are missing the x^4 fit parameter factor)

I've been using pol4 for the fitting the dcol eff vs inst. lumi. plot, and I've fixed the parameter 0 to 1 (to have 100% eff at 0 inst. lumi). And when I was printing out the efficiency factors, I was using the number GetNumberFreeParameters() to loop on all parameters. But since I've fixed 1 parameters, this was missing the last parameter...
I should've realized my mistake at many occasions, sorry...

So this validation is useless, since the function with only 4 parameters looks like this
image

@mmusich
Copy link

mmusich commented Feb 21, 2023

@bartokm

So this validation is useless, since the function with only 4 parameters looks like this

this explains why the new efficiency is better than the old one.

I posted earlier in the thread a plot from the PI about the PU factors (see #1 (comment)). From there you can see there are only 4 parameters. Why wasn't it noticed before?

@bartokm
Copy link

bartokm commented Feb 21, 2023

I've checked your plot, but I've just didn't realize there is 1 factor missing. I'm sorry.

@sroychow
Copy link
Contributor Author

sroychow commented Feb 21, 2023

@bartokm @ferencek @mroguljic
In the discussion from last week, it was suggested that we validate the payloads(both with and without module dependence).
I propose the following action items for today

  • Fix this version of the payload
  • Request new GT
  • Request relval samples again (might be possible to reuse the GEN-SIM)

For tomorrow, we discuss the results for module dependency.

@bartokm
Copy link

bartokm commented Feb 21, 2023

/afs/cern.ch/user/m/mbartok/public/pixel_dynamic_inefficiency_payloads/2023/validation/SiPixelDynamicInefficiency_PhaseI_2023_v1_fix.db

@mmusich
Copy link

mmusich commented Feb 21, 2023

/afs/cern.ch/user/m/mbartok/public/pixel_dynamic_inefficiency_payloads/2023/validation/SiPixelDynamicInefficiency_PhaseI_2023_v1_fix.db

and here as an appetizer is the plot for this payload:

image

(working on the plot for the polynomial representation)

@silviodonato
Copy link

silviodonato commented Feb 21, 2023

Hi @sroychow @bartokm,
do I understand correctly that this bug affects only the SiPixelDynamicInefficiency that you wanted to include in the reDigi of the Run3Winter23?

I mean the current 12_6_X GEN-SIM-RAW should be "ok"
/SinglePionGun_E0p2to200/Run3Winter23Digi-EpsilonPU_126X_mcRun3_2023_forPU65_v1-v1/GEN-SIM-RAW
(they should not have the SiPixelDynamicInefficiency at all)

PS. 126X_mcRun3_2023_forPU65_v1 contains SiPixelDynamicInefficiency_PhaseI_v9 as SiPixelDynamicInefficiencyRcd (no idea of what it means, I only know that the payload was inserted on 2021-07-07)

@mmusich
Copy link

mmusich commented Feb 21, 2023

@silviodonato

(they should not have the SiPixelDynamicInefficiency at all)

they do have it, but it's the "naive" linear extrapolation from the Run-2 pre-kink data, which should be underestimating the real inefficiency at high PU.

@SanjanaSekhar
Copy link

@sroychow the new GT has been created.

@mmusich
Copy link

mmusich commented Feb 22, 2023

(working on the plot for the polynomial representation)

still a bit rough at the edges (need to map the packed DetId to a human readable location in the detector), but here are the validation plots:

  • SiPixelDynamicInefficiency_phase1_2023_v1_fix (new fixed payload):
    image

  • SiPixelDynamicInefficiency_phase1_2023_v1 (bugged payload):
    image

  • SiPixelDynamicInefficiency_PhaseI_v9 (for reference the old payload currently in GlobalTag):
    image

@sroychow
Copy link
Contributor Author

New payload

  • /afs/cern.ch/user/m/mbartok/public/pixel_dynamic_inefficiency_payloads/2023/validation/SiPixelDynamicInefficiency_PhaseI_2023_v2.db

@tvami
Copy link

tvami commented Feb 22, 2023

New GT: 124X_mcRun3_2022_realistic_postEE_forPixelIneff_v3

@mmusich
Copy link

mmusich commented Feb 23, 2023

/afs/cern.ch/user/m/mbartok/public/pixel_dynamic_inefficiency_payloads/2023/validation/SiPixelDynamicInefficiency_PhaseI_2023_v2.db

I checked the polynomial representation:

image

barring some mistakes on my side, layer2 (i.e. region3 in the plot) looks fishy?

@bartokm
Copy link

bartokm commented Feb 23, 2023

Indeed it looks bad... Let me double check

@mmusich
Copy link

mmusich commented Mar 7, 2023

and it would seem it has inner and outer modules swapped in the same way SiPixelDynamicInefficiency_PhaseI_v9 does

I guess it only proves the payloads have always been bugged :D

@ferencek
Copy link
Collaborator

ferencek commented Mar 7, 2023

Just a few days ago when Matej and I were discussing with Marton DetIds for inner and outer modules and I was looking for something like that file but couldn't find it. I have http://dkotlins.web.cern.ch/dkotlins/CMS/ bookmarked but it never occurred to me to check inside the MyDocs subfolder :) I

my understanding is that the code to prepare these files is here (source, config)

I had a quick chat with Tanja and she reminded me that the Phase 1 geometry was initially buggy so I re-run SiPixelDets.py in CMSSW_12_4_0 using the auto:phase1_2022_design GT and here are the first few lines from the output

303042564 BPix_BmI_SEC3_LYR1_LDR3F_MOD4 r/phi/z = 3.05606/0.277804/-23.45 cmssw layer/ladder/module 1/1/1 E-phi 0.277804
303042568 BPix_BmI_SEC4_LYR1_LDR3F_MOD3 r/phi/z = 3.05606/0.277804/-16.75 cmssw layer/ladder/module 1/1/2 E-phi 0.277804
303042572 BPix_BmI_SEC4_LYR1_LDR3F_MOD2 r/phi/z = 3.05606/0.277804/-10.05 cmssw layer/ladder/module 1/1/3 E-phi 0.277804
303042576 BPix_BmI_SEC4_LYR1_LDR3F_MOD1 r/phi/z = 3.05606/0.277804/-3.35 cmssw layer/ladder/module 1/1/4 E-phi 0.277804
303042580 BPix_BpI_SEC4_LYR1_LDR3F_MOD1 r/phi/z = 3.05606/0.277804/3.35 cmssw layer/ladder/module 1/1/5 E-phi 0.277804
303042584 BPix_BpI_SEC4_LYR1_LDR3F_MOD2 r/phi/z = 3.05606/0.277804/10.05 cmssw layer/ladder/module 1/1/6 E-phi 0.277804
303042588 BPix_BpI_SEC4_LYR1_LDR3F_MOD3 r/phi/z = 3.05606/0.277804/16.75 cmssw layer/ladder/module 1/1/7 E-phi 0.277804
303042592 BPix_BpI_SEC3_LYR1_LDR3F_MOD4 r/phi/z = 3.05606/0.277804/23.45 cmssw layer/ladder/module 1/1/8 E-phi 0.277804
303046660 BPix_BmI_SEC2_LYR1_LDR2F_MOD4 r/phi/z = 2.73876/0.801403/-23.45 cmssw layer/ladder/module 1/2/1 E-phi -2.34019
303046664 BPix_BmI_SEC2_LYR1_LDR2F_MOD3 r/phi/z = 2.73876/0.801403/-16.75 cmssw layer/ladder/module 1/2/2 E-phi -2.34019
303046668 BPix_BmI_SEC3_LYR1_LDR2F_MOD2 r/phi/z = 2.73876/0.801403/-10.05 cmssw layer/ladder/module 1/2/3 E-phi -2.34019
303046672 BPix_BmI_SEC3_LYR1_LDR2F_MOD1 r/phi/z = 2.73876/0.801403/-3.35 cmssw layer/ladder/module 1/2/4 E-phi -2.34019
303046676 BPix_BpI_SEC3_LYR1_LDR2F_MOD1 r/phi/z = 2.73876/0.801403/3.35 cmssw layer/ladder/module 1/2/5 E-phi -2.34019
303046680 BPix_BpI_SEC3_LYR1_LDR2F_MOD2 r/phi/z = 2.73876/0.801403/10.05 cmssw layer/ladder/module 1/2/6 E-phi -2.34019
303046684 BPix_BpI_SEC2_LYR1_LDR2F_MOD3 r/phi/z = 2.73876/0.801403/16.75 cmssw layer/ladder/module 1/2/7 E-phi -2.34019
303046688 BPix_BpI_SEC2_LYR1_LDR2F_MOD4 r/phi/z = 2.73876/0.801403/23.45 cmssw layer/ladder/module 1/2/8 E-phi -2.34019

which is in line with the pixel hadrography plot and different from Danek's file (the timestamp of Danek's file is from 2016 so very likely corresponds to this initial buggy geometry). This means that the latest dynamic inefficiency payload is indeed buggy and we will need to re-run the RelVal production. @mmusich, I guess this also means that the PayloadInspector will need to be updated.

A fixed dynamic inefficiency payload is available here /afs/cern.ch/user/m/mbartok/public/pixel_dynamic_inefficiency_payloads/2023/validation/SiPixelDynamicInefficiency_PhaseI_2023_v2_fix2.db @SanjanaSekhar, can you please take care of uploading it to the DB and requesting a new GT (I guess v5). Thanks a lot.

So I guess what's left to be understood is the ladder numbering in the DQM plots.

@mmusich
Copy link

mmusich commented Mar 7, 2023

I guess it only proves the payloads have always been bugged :D

I've re-run the analyzer in a fresh Run-3 setup and now I get completely opposite conclusions: http://musich.web.cern.ch/musich/display/SiPixelPhase1Dets.txt

my printouts:

region name: BPix L1/i has the following modules attached: [303042564, 303042568, 303042572, 303042576, 303042580, 303042584, 303042588, 303042592, 303050756, 303050760, 303050764, 303050768, 303050772, 303050776, 303050780, 303050784, 303058948, 303058952, 303058956, 303058960, 303058964, 303058968, 303058972, 303058976, 303067140, 303067144, 303067148, 303067152, 303067156, 303067160, 303067164, 303067168, 303075332, 303075336, 303075340, 303075344, 303075348, 303075352, 303075356, 303075360, 303083524, 303083528, 303083532, 303083536, 303083540, 303083544, 303083548, 303083552]  has representation (1, -0.0110191, -0.000213581, 8.39581e-05, -3.04353e-06) 


region name: BPix L1/o has the following modules attached: [303046660, 303046664, 303046668, 303046672, 303046676, 303046680, 303046684, 303046688, 303054852, 303054856, 303054860, 303054864, 303054868, 303054872, 303054876, 303054880, 303063044, 303063048, 303063052, 303063056, 303063060, 303063064, 303063068, 303063072, 303071236, 303071240, 303071244, 303071248, 303071252, 303071256, 303071260, 303071264, 303079428, 303079432, 303079436, 303079440, 303079444, 303079448, 303079452, 303079456, 303087620, 303087624, 303087628, 303087632, 303087636, 303087640, 303087644, 303087648]  has representation (1, -0.00470212, -0.000869728, 9.87286e-05, -2.65723e-06) 

don't match at all.

@mmusich
Copy link

mmusich commented Mar 7, 2023

@ferencek

303042564 BPix_BmI_SEC3_LYR1_LDR3F_MOD4 r/phi/z = 3.05606/0.277804/-23.45 cmssw layer/ladder/module 1/1/1 E-phi 0.277804
303042568 BPix_BmI_SEC4_LYR1_LDR3F_MOD3 r/phi/z = 3.05606/0.277804/-16.75 cmssw layer/ladder/module 1/1/2 E-phi 0.277804
303042572 BPix_BmI_SEC4_LYR1_LDR3F_MOD2 r/phi/z = 3.05606/0.277804/-10.05 cmssw layer/ladder/module 1/1/3 E-phi 0.277804
303042576 BPix_BmI_SEC4_LYR1_LDR3F_MOD1 r/phi/z = 3.05606/0.277804/-3.35 cmssw layer/ladder/module 1/1/4 E-phi 0.277804
303042580 BPix_BpI_SEC4_LYR1_LDR3F_MOD1 r/phi/z = 3.05606/0.277804/3.35 cmssw layer/ladder/module 1/1/5 E-phi 0.277804
303042584 BPix_BpI_SEC4_LYR1_LDR3F_MOD2 r/phi/z = 3.05606/0.277804/10.05 cmssw layer/ladder/module 1/1/6 E-phi 0.277804
303042588 BPix_BpI_SEC4_LYR1_LDR3F_MOD3 r/phi/z = 3.05606/0.277804/16.75 cmssw layer/ladder/module 1/1/7 E-phi 0.277804
303042592 BPix_BpI_SEC3_LYR1_LDR3F_MOD4 r/phi/z = 3.05606/0.277804/23.45 cmssw layer/ladder/module 1/1/8 E-phi 0.277804
303046660 BPix_BmI_SEC2_LYR1_LDR2F_MOD4 r/phi/z = 2.73876/0.801403/-23.45 cmssw layer/ladder/module 1/2/1 E-phi -2.34019
303046664 BPix_BmI_SEC2_LYR1_LDR2F_MOD3 r/phi/z = 2.73876/0.801403/-16.75 cmssw layer/ladder/module 1/2/2 E-phi -2.34019
303046668 BPix_BmI_SEC3_LYR1_LDR2F_MOD2 r/phi/z = 2.73876/0.801403/-10.05 cmssw layer/ladder/module 1/2/3 E-phi -2.34019
303046672 BPix_BmI_SEC3_LYR1_LDR2F_MOD1 r/phi/z = 2.73876/0.801403/-3.35 cmssw layer/ladder/module 1/2/4 E-phi -2.34019
303046676 BPix_BpI_SEC3_LYR1_LDR2F_MOD1 r/phi/z = 2.73876/0.801403/3.35 cmssw layer/ladder/module 1/2/5 E-phi -2.34019
303046680 BPix_BpI_SEC3_LYR1_LDR2F_MOD2 r/phi/z = 2.73876/0.801403/10.05 cmssw layer/ladder/module 1/2/6 E-phi -2.34019
303046684 BPix_BpI_SEC2_LYR1_LDR2F_MOD3 r/phi/z = 2.73876/0.801403/16.75 cmssw layer/ladder/module 1/2/7 E-phi -2.34019
303046688 BPix_BpI_SEC2_LYR1_LDR2F_MOD4 r/phi/z = 2.73876/0.801403/23.45 cmssw layer/ladder/module 1/2/8 E-phi -2.34019

according to this printout, odd ladders (e.g. 1) are outer while even ladders (e.g. 2) are inner.
This makes my code at SiPixelPayloadInspectorHelper.h#L635-L654 wrong (at least for Phase-1, I haven't checked yet Phase-0):

inline bool isBPixOuterLadder(const DetId& detid, const TrackerTopology& tTopo, bool isPhase0)
  /*--------------------------------------------------------------------*/
  {
    bool isOuter = false;
    int layer = tTopo.pxbLayer(detid.rawId());
    bool odd_ladder = tTopo.pxbLadder(detid.rawId()) % 2;
    if (isPhase0) {
      if (layer == 2)
        isOuter = !odd_ladder;
      else
        isOuter = odd_ladder;
    } else {
      if (layer == 4)
        isOuter = odd_ladder;
      else
        isOuter = !odd_ladder;
    }
    return isOuter;
  }

this code was copied from Janos' one here in SiPixelCoordinates.cc#L155-L180

// Using TrackerTopology
// Ladders have a staggered structure
// Non-flipped ladders are on the outer radius
// Phase 0: Outer ladders are odd for layer 1,3 and even for layer 2
// Phase 1: Outer ladders are odd for layer 4 and even for layer 1,2,3
int SiPixelCoordinates::outer(const DetId& detid) {
  if (outer_.count(detid.rawId()))
    return outer_[detid.rawId()];
  if (!isBPix_(detid))
    return outer_[detid.rawId()] = -9999;
  int outer = -9999;
  int layer = tTopo_->pxbLayer(detid.rawId());
  bool odd_ladder = tTopo_->pxbLadder(detid.rawId()) % 2;
  if (phase_ == 0) {
    if (layer == 2)
      outer = !odd_ladder;
    else
      outer = odd_ladder;
  } else if (phase_ == 1) {
    if (layer == 4)
      outer = odd_ladder;
    else
      outer = !odd_ladder;
  }
  return outer_[detid.rawId()] = outer;
}

which is then equally wrong.

So I guess what's left to be understood is the ladder numbering in the DQM plots.

Since SiPixelCoordinates AFAIK is widely used in the Phase1 pixel DQM this is worrisome and perhaps can explain some of the discrepancy?

On the bright side, this other piece of code here SiPixelPhase1TrackClusters.cc#L209-L212

        bool iAmOuter = ((tkTpl->pxbLadder(id) % 2 == 1) && tkTpl->pxbLayer(id) != 4) ||
                        ((tkTpl->pxbLadder(id) % 2 != 1) && tkTpl->pxbLayer(id) == 4);

should be right.

I had a quick chat with Tanja and she reminded me that the Phase 1 geometry was initially buggy

I am speculating if the DQM code was originally written using as a guideline the buggy phase-1 geometry prior to its fixing.

@SanjanaSekhar
Copy link

New GT: 124X_mcRun3_2022_realistic_postEE_forPixelIneff_v5

@ferencek
Copy link
Collaborator

ferencek commented Mar 7, 2023

this code was copied from Janos' one here in SiPixelCoordinates.cc#L155-L180

// Using TrackerTopology
// Ladders have a staggered structure
// Non-flipped ladders are on the outer radius
// Phase 0: Outer ladders are odd for layer 1,3 and even for layer 2
// Phase 1: Outer ladders are odd for layer 4 and even for layer 1,2,3
int SiPixelCoordinates::outer(const DetId& detid) {
  if (outer_.count(detid.rawId()))
    return outer_[detid.rawId()];
  if (!isBPix_(detid))
    return outer_[detid.rawId()] = -9999;
  int outer = -9999;
  int layer = tTopo_->pxbLayer(detid.rawId());
  bool odd_ladder = tTopo_->pxbLadder(detid.rawId()) % 2;
  if (phase_ == 0) {
    if (layer == 2)
      outer = !odd_ladder;
    else
      outer = odd_ladder;
  } else if (phase_ == 1) {
    if (layer == 4)
      outer = odd_ladder;
    else
      outer = !odd_ladder;
  }
  return outer_[detid.rawId()] = outer;
}

which is then equally wrong.

From the history on GitHub I see that this file was created in January of 2017 at which point the problem with the geometry was possibly not yet known. Nevertheless, it looks like the code was never fixed.

So I guess what's left to be understood is the ladder numbering in the DQM plots.

Since SiPixelCoordinates AFAIK is widely used in the Phase1 pixel DQM this is worrisome and perhaps can explain some of the discrepancy?

Indeed, there could be several places in DQM where things are not handled correctly due to buggy SiPixelCoordinates but the problem I pointed out would not be solved with the correct inner/outer assignment because those plots suggest that inner and outer ladders are not always alternating and instead ladders 1 and -1 in the online convention (or 3 and 4, respectively, in the offline convention) are of the same type which is definitely not the case.

On the bright side, this other piece of code here SiPixelPhase1TrackClusters.cc#L209-L212

        bool iAmOuter = ((tkTpl->pxbLadder(id) % 2 == 1) && tkTpl->pxbLayer(id) != 4) ||
                        ((tkTpl->pxbLadder(id) % 2 != 1) && tkTpl->pxbLayer(id) == 4);

should be right.

It looks like this was fixed in July of 2017 before the file was moved to its current location.

I had a quick chat with Tanja and she reminded me that the Phase 1 geometry was initially buggy

I am speculating if the DQM code was originally written using as a guideline the buggy phase-1 geometry prior to its fixing.

This is very much possible and, unfortunately, some parts of the DQM code possibly never got fixed later on.

This made me wonder if handling of the flipped and unflipped modules could have also been affected and from a quick search at least one place in the alignment code is working with the wrong geometry.

@mmusich
Copy link

mmusich commented Mar 8, 2023

From the history on GitHub I see that this file was created in January of 2017 at which point the problem with the geometry was possibly not yet known. Nevertheless, it looks like the code was never fixed.

looks like it's time to fix it. I am wondering if the same bug is present in other critical pieces pixel offline s/w not in cmssw. We might be only scratching the surface here.

but the problem I pointed out would not be solved with the correct inner/outer assignment because those plots suggest that inner and outer ladders are not always alternating and instead ladders 1 and -1 in the online convention (or 3 and 4, respectively, in the offline convention) are of the same type which is definitely not the case.

as far as I understand the code for assigning the signed ladder is again in SiPixelCoordiates, namely here SiPixelCoordinates.cc#L120-L131

int SiPixelCoordinates::signed_ladder(const DetId& detid) {
  if (signed_ladder_.count(detid.rawId()))
    return signed_ladder_[detid.rawId()];
  if (!isBPix_(detid))
    return signed_ladder_[detid.rawId()] = -9999;
  int signed_ladder = PixelBarrelName(detid, tTopo_, phase_).ladderName();
  if (quadrant(detid) % 2)
    signed_ladder *= -1;
  return signed_ladder_[detid.rawId()] = signed_ladder;
}

you might want to cross-check if it conforms with your expectations.

This made me wonder if handling of the flipped and unflipped modules could have also been affected and from a quick search at least one place in the alignment code is working with the wrong geometry.

fortunately enough, the line you point resides in a not really critical piece of ancillary s/w inside a functionality that was never (afaik) used. On the other hand there are other points in cmssw that exploit the radius <-> flipping relationship.
Before I go and fix all this mess left from the forebears, my understanding from the pixel hadrography plot is that the right set of relationships (for Phase-1) is:

  • odd ladder = inner = flipped (for BPix 4)
  • even ladder = inner = flipped (for BPix 1,2,3)

please confirm.

@ferencek
Copy link
Collaborator

ferencek commented Mar 8, 2023

@mmusich

* odd ladder = inner = flipped (for BPix 4)

* even ladder = inner = flipped (for BPix 1,2,3)

please confirm.

This is correct.

@ferencek
Copy link
Collaborator

ferencek commented Mar 8, 2023

as far as I understand the code for assigning the signed ladder is again in SiPixelCoordiates, namely here SiPixelCoordinates.cc#L120-L131

int SiPixelCoordinates::signed_ladder(const DetId& detid) {
  if (signed_ladder_.count(detid.rawId()))
    return signed_ladder_[detid.rawId()];
  if (!isBPix_(detid))
    return signed_ladder_[detid.rawId()] = -9999;
  int signed_ladder = PixelBarrelName(detid, tTopo_, phase_).ladderName();
  if (quadrant(detid) % 2)
    signed_ladder *= -1;
  return signed_ladder_[detid.rawId()] = signed_ladder;
}

you might want to cross-check if it conforms with your expectations.

From the code itself this looks fine but I also ran some tests and it works as expected.

@sroychow
Copy link
Contributor Author

@bartokm @Tiziano @ferencek @mroguljic
Samples production can be tracked here:-

For the reference, the ALCARECO has quite a sizeable stat. Can you start on the analysis already?

@mroguljic
Copy link
Collaborator

@bartokm it looks like the target dataset is also ready. Could you submit jobs for it as well?

@bartokm
Copy link

bartokm commented Apr 26, 2023

Following the discussion at yesterday's Pixel Offline meeting, I've prepared a new version of the payload: updating L1 and L2 factors, using the new hit efficiency vs double column efficiency function.
/afs/cern.ch/user/m/mbartok/public/pixel_dynamic_inefficiency_payloads/2023/validation/SiPixelDynamicInefficiency_PhaseI_2023_v6.db

@sroychow
Copy link
Contributor Author

Payload has been uploaded as SiPixelDynamicInefficiency_phase1_2023_v2_fix3. Plots from PI
image
image

@mmusich
Copy link

mmusich commented Apr 28, 2023

Plots from PI

there's something wrong again (either in the plot or in the payload): BPix1 / outer has lower efficiency than BPix1 / inner.

@mmusich
Copy link

mmusich commented Apr 28, 2023

there's something wrong again (either in the plot or in the payload): BPix1 / outer has lower efficiency than BPix1 / inner.

indeed, when generating the plot in the PI , I see a different picture: https://cern.ch/go/WN8n

@sroychow
Copy link
Contributor Author

@mmusich this link doesn't work for me

@mmusich
Copy link

mmusich commented Apr 28, 2023

@sroychow there is a problem with the short URL in the service, try now.

@mmusich
Copy link

mmusich commented Apr 28, 2023

anyhow @sroychow it begs the question how did you generate the wrong plot

@mroguljic
Copy link
Collaborator

mroguljic commented Apr 28, 2023

Looks like it's due to CMSSW version of PI: (https://cern.ch/go/Lq8d)
EDIT: Full URL

@sroychow
Copy link
Contributor Author

anyhow @sroychow it begs the question how did you generate the wrong plot

I selected 13_1_0_pre1 , and got this https://cern.ch/go/Lq8d

@mmusich
Copy link

mmusich commented Apr 28, 2023

Looks like it's due to CMSSW version of PI: https://cern.ch/go/Lq8d

this link doens't work, anyhow you should always choose the latest version available (in the case at hand it seems you are using a version predating the integration of cms-sw/cmssw#40989

@mmusich
Copy link

mmusich commented Apr 28, 2023

I selected 13_1_0_pre1 , and got this https://cern.ch/go/Lq8d

you all keep making the same mistake I did (please don't use the short URL, until it's fixed)

@francescobrivio
Copy link

@sroychow there is a problem with the short URL in the service, try now.

Indeed it seems a bug in the shortening service. We are looking into this (I made CMSDBBROWS-138 to keep track).
In the meanwhile please use the full url.

@sroychow
Copy link
Contributor Author

Looks like it's due to CMSSW version of PI: https://cern.ch/go/Lq8d

this link doens't work, anyhow you should always choose the latest version available (in the case at hand it seems you are using a version predating the integration of cms-sw/cmssw#40989

right understood, thanks.. sorry for the confusion
@mroguljic can you proceed with the request for the GT then?

@mroguljic
Copy link
Collaborator

@sroychow Requested at CMS-talk thread

@mmusich
Copy link

mmusich commented Apr 28, 2023

here is for the record a new version of the plot with some improvement (inner / outer fully spelled out and fixed y-scale as recently requested privately):

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants