CMSTrackerDPG / Tasks

0 stars 0 forks source link

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

Closed sroychow closed 7 months ago

sroychow commented 1 year ago

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

mmusich commented 1 year ago

that apparently makes the difference, because I am able to reproduce locally the bugged behaviour. Funky that it depends on the architecture :/

Actually it seems it's a problem of getting the library from cvmfs: On a lxplus8 node:

setenv SCRAM_ARCH el8_amd64_gcc11 
cd /cvmfs/cms.cern.ch/${SCRAM_ARCH}/cms/cmssw/CMSSW_13_1_0_pre1/src
cmsenv
cd -
getPayloadData.py --plugin pluginSiPixelDynamicInefficiency_PayloadInspector --plot plot_SiPixelDynamicInefficiencyPUParametrization --tag SiPixelDynamicInefficiency_phase1_2023_v1_fix --time_type Run --iovs '{"start_iov": "1", "end_iov": "1"}' --db Prod --test ;

leads to https://musich.web.cern.ch/musich/display/el8_CMSSW_13_1_0_pre1.png

cd /cvmfs/cms-ib.cern.ch/sw/x86_64/week1/el8_amd64_gcc11/cms/cmssw/CMSSW_13_1_X_2023-03-06-2300/src/
cmsenv
cd -
getPayloadData.py --plugin pluginSiPixelDynamicInefficiency_PayloadInspector --plot plot_SiPixelDynamicInefficiencyPUParametrization --tag SiPixelDynamicInefficiency_phase1_2023_v1_fix --time_type Run --iovs '{"start_iov": "1", "end_iov": "1"}' --db Prod --test ;

leads to https://musich.web.cern.ch/musich/display/el8_CMSSW_13_1_X_2023-03-06-2300.png

but recompiling the package locally with:

setenv SCRAM_ARCH el8_amd64_gcc11
cmsrel CMSSW_13_1_X_2023-03-06-2300
cd CMSSW_13_1_X_2023-03-06-2300/src
git cms-addpkg CondCore/SiPixelPlugins
scram b -j 20
getPayloadData.py --plugin pluginSiPixelDynamicInefficiency_PayloadInspector --plot plot_SiPixelDynamicInefficiencyPUParametrization --tag SiPixelDynamicInefficiency_phase1_2023_v1_fix --time_type Run --iovs '{"start_iov": "1", "end_iov": "1"}' --db Prod --test ;

leads to https://musich.web.cern.ch/musich/display/el8_CMSSW_13_1_X_2023-03-06-2300_fromLocal.png

mmusich commented 1 year ago

@ferencek @mroguljic

I can easily check if I am assigning the label correctly I you can give me a list of raw Ids for Layer1 internal / external ladders which come from an undisputed source (I was using some logic in the DQM code to determine if the ladders are inner or outer:

I tend to believe that my current implementation in the payload inspector of the inner / outer ladders is correct. If I plug-in the following debug statement:

diff --git a/CondCore/SiPixelPlugins/plugins/SiPixelDynamicInefficiency_PayloadInspector.cc b/CondCore/SiPixelPlugins/plugins/SiPixelDynamicInefficiency_PayloadInspector.cc
index 1804c126adb..6e47885f7ec 100644
--- a/CondCore/SiPixelPlugins/plugins/SiPixelDynamicInefficiency_PayloadInspector.cc
+++ b/CondCore/SiPixelPlugins/plugins/SiPixelDynamicInefficiency_PayloadInspector.cc
@@ -797,6 +797,12 @@ namespace {
           auto PhInfo = SiPixelPI::PhaseInfo(SiPixelPI::phase1size);
           const auto& regName = this->attachLocationLabel(modules, PhInfo);
           namesOfParts.push_back(regName);
+
+          std::cout << "region name: " << regName << " has the following modules attached: ";
+          for (const auto& module : modules) {
+            std::cout << module << ", ";
+          }
+          std::cout << "\n" << std::endl;
         }

I obtain the following lists:

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, 

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, 

cross-checking the radius of these DetIds with this file from Danek, I see that the dets in the BPix L1/i list have radius ~ 2.7cm, while the dets in BPix L1/o have radius > 3cm.

I am asking because if 2023_v2_fix is plotted correctly, it would mean that in the old v9 payload factors were not assigned correctly.

this indeed seems to be the case.

ferencek commented 1 year ago

I had a look at an earlier dynamic inefficiency payload, SiPixelDynamicInefficiency_PhaseI_v6, used in the Run 2 legacy MC production (e.g. contained in the 106X_upgrade2018_realistic_v16_L1v1 GT) and it would seem it has inner and outer modules swapped in the same way SiPixelDynamicInefficiency_PhaseI_v9 does SiPixelDynamicInefficiency_PhaseI_v6 PU parametrization

image

Btw, thanks @mmusich for the link to Danek's txt file. 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 :) Instead, I was struggling to figure out DetIds by hand using the DetId schema and an online binary to decimal converter. Eventually I wrote a simple EDAnalyzer that extracts this info from the TrackerTopology record but Danek's txt file is much more comprehensive.

Looking at this txt file, I am now quite puzzled because the geometry it describes is at odds with the plot Matej posted in an earlier comment (this plots is from Fig. 41 on page 53 of The CMS Phase-1 pixel detector upgrade paper).

Regardless of all of this confusion, it looks like there is an additional problem in DQM because the modulation pattern in the cluster size y per ladder in BmI

image should be shifter in phase wrt the cluster size y per ladder in BmO

image

but instead it is not.

mmusich commented 1 year ago

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)

mmusich commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

New GT: 124X_mcRun3_2022_realistic_postEE_forPixelIneff_v5

ferencek commented 1 year ago

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 commented 1 year ago

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:

please confirm.

ferencek commented 1 year ago

@mmusich

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

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

please confirm.

This is correct.

ferencek commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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

bartokm commented 1 year ago

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 commented 1 year ago

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

mmusich commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@mmusich this link doesn't work for me

mmusich commented 1 year ago

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

mmusich commented 1 year ago

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

mroguljic commented 1 year ago

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

sroychow commented 1 year ago

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 commented 1 year ago

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 https://github.com/cms-sw/cmssw/pull/40989

mmusich commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

@sroychow Requested at CMS-talk thread

mmusich commented 1 year ago

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