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

introducing BigPixel for phase-2 InnerTracker in T39 geometry #44576

Merged
merged 10 commits into from
May 21, 2024

Conversation

emiglior
Copy link
Contributor

PR description:

This PR is a follow-up of the draft PR#42454 (closed in Jan 2024).

The PR introduces the so-called BigPixels (elongated pixels between two readout chips) for the phase-2 InnerTracker following the implementation proposed in:

https://indico.cern.ch/event/1382902/contributions/5816585/attachments/2805475/4895325/Phase-2_IT_BigPix_TrkDPG.pdf

  • Description. BigPixels are introduced in the file pixelStructureTopology.xml or by using the TrackerAdditionalParametersPerDet rcd for geometries loaded from DB (none yet). Therefore, the PR restores the changes of the infrastructure needed for using the TrackerAdditionalParametersPerDet rcd, introduced for the bricked pixels and then removed with PR#40443 but without reintroducing the functions related to bricked pixels.
    NOTE (similar to PR#34120): the record PTrackerAdditionalParametersPerDetRcd is consumed by the TrackerGeomBuilderFromGeometricDet, which is the same for Runs 2&3 and Phase 2. Therefore, this PR also modifies Runs 2&3, by including the new record.

  • Tracker geometry. The Tracker phase-2 test geometry in which BigPixels are introduced is T39, thus skipping versions T36-T37-T38 which are currently under development by @Raffaella07.

  • Implementation. Phase-2 pixels are now described by the class RectangularPhase2PixelTopology inheriting from PixelTopology. Code for Tracker local reco was adapted to work both the phase-1 pixels (RectangularPixelTopology) and with phase-2 pixels (RectangularPhase2PixelTopology). The function siPixelUtils::generic_position_formula(), used in PixelCPEGeneric, was modified in order to be used both for phase-1 and phase-2 BigPixels. A side-effect was to add to the abstract class PixelTopology the methods getPixelFractionInX() and getPixelFractionInY(). @fabiocos @martinamalberti: the implementation of these fcn in ProxyMTDTopology is just a placeholder to compile the code. Please let us know what the actual code should be.

PR validation:

compilation ok after git cms-checkdeps -a -A
scram b code-checks, code-format
scram b runtests
runTheMatrix.py -l limited -i all --ibeos

No changes are expected on phase-1 (set of tracking validation plots on a single muon sample on phase-1 geometry)
http://personalpages.to.infn.it/~migliore/BigPixels/SingleMuPt100-phase1/plots-20240323/

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:

This PR is not a backport and no backport needed.

Specific w/f for test on T39: 30034.0

@gbardell @langufo @bartosik-hep
@mmusich @sroychow @mdelcourt FYI

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 28, 2024

cms-bot internal usage

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44576/39731

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

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44576/39732

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

  • Alignment/CommonAlignmentMonitor (alca)
  • Alignment/CommonAlignmentProducer (alca)
  • Alignment/LaserAlignment (alca)
  • Alignment/MillePedeAlignmentAlgorithm (alca)
  • Alignment/OfflineValidation (alca)
  • Alignment/SurveyAnalysis (alca)
  • Alignment/TrackerAlignment (alca)
  • CondFormats/GeometryObjects (alca, db)
  • CondFormats/SiPixelTransient (reconstruction, db)
  • CondTools/Geometry (db)
  • Configuration/Geometry (geometry, upgrade)
  • Configuration/PyReleaseValidation (pdmv, upgrade)
  • Configuration/StandardSequences (operations)
  • DQM/SiTrackerPhase2 (dqm)
  • Geometry/CMSCommonData (geometry, upgrade)
  • Geometry/CommonTopologies (geometry)
  • Geometry/MTDGeometryBuilder (geometry, upgrade)
  • Geometry/TrackerCommonData (geometry)
  • Geometry/TrackerGeometryBuilder (geometry)
  • Geometry/TrackerNumberingBuilder (geometry)
  • HLTrigger/Configuration (hlt)
  • L1Trigger/L1TTrackMatch (upgrade, l1)
  • L1Trigger/TrackFindingTracklet (l1)
  • L1Trigger/TrackTrigger (upgrade, l1)
  • RecoHI/HiTracking (reconstruction)
  • RecoLocalTracker/SiPixelRecHits (reconstruction)
  • RecoLocalTracker/SubCollectionProducers (reconstruction)
  • RecoLuminosity/LumiProducer (reconstruction)
  • RecoTracker/PixelLowPtUtilities (reconstruction)
  • SLHCUpgradeSimulations/Geometry (geometry, upgrade)

@sunilUIET, @rvenditti, @tjavaid, @AdrianoDee, @perrotta, @subirsarkar, @bsunanda, @civanch, @saumyaphor4252, @mmusich, @mandrenguyen, @consuegs, @Dr15Jones, @fabiocos, @aloeliger, @Martin-Grunewald, @davidlange6, @epalencia, @srimanob, @cmsbuild, @rappoccio, @mdhildreth, @nothingface0, @antoniovilela, @miquork, @antoniovagnerini, @francescobrivio, @jfernan2, @syuvivida, @makortel can you please review it and eventually sign? Thanks.
@jazzitup, @ferencek, @vargasa, @mtosi, @yduhm, @ghugo83, @sroychow, @threus, @slomeo, @VourMa, @gbenelli, @alesaggio, @mroguljic, @venturia, @sviret, @tlampen, @GiacomoSguazzoni, @bsunanda, @seemasharmafnal, @gpetruc, @skinnari, @arossi83, @mmusich, @mandrenguyen, @VinInn, @felicepantaleo, @tocheng, @silviodonato, @erikbutz, @fabiocos, @JanChyczynski, @echabert, @tsusa, @sameasy, @pakhotin, @Martin-Grunewald, @yuanchao, @missirol, @dgulhan, @yetkinyilmaz, @jlidrych, @robervalwalsh, @tvami, @rovere, @kurtejung, @PonIlya, @JanFSchulte, @rsreds, @dkotlins, @yenjie, @adewit, @makortel this is something you requested to watch as well.
@sextonkennedy, @rappoccio, @antoniovilela you are the release manager for this.

cms-bot commands are listed here

@mmusich
Copy link
Contributor

mmusich commented Mar 29, 2024

@cmsbuild, please test

@mmusich
Copy link
Contributor

mmusich commented May 8, 2024

@cmsbuild, please test

  • failures are transient

@cmsbuild
Copy link
Contributor

cmsbuild commented May 8, 2024

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3f5fe0/39293/summary.html
COMMIT: 01b4af5
CMSSW: CMSSW_14_1_X_2024-05-07-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/44576/39293/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially added 1 lines to the logs
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 4844 differences found in the comparisons
  • DQMHistoTests: Total files compared: 49
  • DQMHistoTests: Total histograms compared: 3438984
  • DQMHistoTests: Total failures: 11754
  • DQMHistoTests: Total nulls: 148
  • DQMHistoTests: Total successes: 3427062
  • DQMHistoTests: Total skipped: 20
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 221968.83 KiB( 48 files compared)
  • DQMHistoSizes: changed ( 23234.0,... ): 36994.805 KiB TrackerPhase2ITDigi/DigiMonitor
  • Checked 206 log files, 170 edm output root files, 49 DQM output files
  • TriggerResults: found differences in 1 / 47 workflows

@mmusich
Copy link
Contributor

mmusich commented May 8, 2024

Reco comparison results: 4844 differences found in the comparisons
[...]
DQMHistoTests: Total failures: 11754
[...]
DQMHistoSizes: changed ( 23234.0,... ): 36994.805 KiB TrackerPhase2ITDigi/DigiMonitor
[...]
TriggerResults: found differences in 1 / 47 workflows

do we understand the origin of these changes?

@emiglior
Copy link
Contributor Author

emiglior commented May 8, 2024

In case of phase-2 w/f, we changed the binning of the th2 digi-map
DQM/SiTrackerPhase2/python/Phase2TrackerMonitorDigi_cff.py
to better spot the presence of big pixels

@mmusich
Copy link
Contributor

mmusich commented May 8, 2024

In case of phase-2 w/f, we changed the binning of the th2 digi-map

this explains this.

DQMHistoSizes: changed ( 23234.0,... ): 36994.805 KiB TrackerPhase2ITDigi/DigiMonitor

what about the other failures?

@emiglior
Copy link
Contributor Author

emiglior commented May 8, 2024

Still on ph-2 w/f, IIUC the failures are actually fired by tiny differences (but let me know if I am wrong). We observed something similar during the development of the code.
We ascribed the effect to the re-coding of the functions

localX(), localY(), pixel()

in Geometry/TrackerGeometryBuilder/src/RectangularPixelPhase2Topology.cc (e.g. different rounding-off's)

@emiglior
Copy link
Contributor Author

hi, any news on the missing signatures?

@rappoccio
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

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 be automatically merged.

@cmsbuild cmsbuild merged commit 47a226a into cms-sw:master May 21, 2024
12 checks passed
@mmusich
Copy link
Contributor

mmusich commented May 22, 2024

@emiglior looks like this PR causes unit tests failures, see e.g. here

@mmusich
Copy link
Contributor

mmusich commented May 22, 2024

looks like this PR causes unit tests failures.

#45013 should fix.

@dan131riley
Copy link

DBG builds are broken, see for instance https://cmssdt.cern.ch/SDT/cgi-bin/buildlogs/el8_amd64_gcc12/CMSSW_14_1_DBG_X_2024-05-23-2300/Geometry/TrackerGeometryBuilder

>> Compiling  src/Geometry/TrackerGeometryBuilder/src/trackerHierarchy.cc
/data/cmsbld/jenkins/workspace/build-any-ib/w/el8_amd64_gcc12/external/gcc/12.3.1-40d504be6370b5a30e3947a6e575ca28/bin/c++ -c -DGNU_GCC -D_GNU_SOURCE -DTBB_USE_GLIBCXX_VERSION=120301 -DTBB_SUPPRESS_DEPRECATED_MESSAGES -DTBB_PREVIEW_RESUMABLE_TASKS=1 -DTBB_PREVIEW_TASK_GROUP_EXTENSIONS=1 -DBOOST_SPIRIT_THREADSAFE -DPHOENIX_THREADSAFE -DBOOST_MATH_DISABLE_STD_FPCLASSIFY -DBOOST_UUID_RANDOM_PROVIDER_FORCE_POSIX -DDD4HEP_USE_GEANT4_UNITS=1 -DCMSSW_GIT_HASH='CMSSW_14_1_DBG_X_2024-05-23-2300' -DPROJECT_NAME='CMSSW' -DPROJECT_VERSION='CMSSW_14_1_DBG_X_2024-05-23-2300' -Isrc -I/data/cmsbld/jenkins/workspace/build-any-ib/w/el8_amd64_gcc12/external/dd4hep/v01-27-02-02d087b69a43429d82df59691bcd25e1/include -I/data/cmsbld/jenkins/workspace/build-any-ib/w/el8_amd64_gcc12/external/pcre/8.43-e34796d17981e9b6d174328c69446455/include -isystem/data/cmsbld/jenkins/workspace/build-any-ib/w/el8_amd64_gcc12/external/boost/1.80.0-941b136a4a3be6f8bc1e903d36ddc172/include -I/data/cmsbld/jenkins/workspace/build-any-ib/w/el8_amd64_gcc12/external/bz2lib/1.0.6-d065ccd79984efc6d4660f410e4c81de/include -isystem/data/cmsbld/jenkins/workspace/build-any-ib/w/el8_amd64_gcc12/external/clhep/2.4.7.1-8e40efd27b7394c1fa4e9c7e432d85cd/include -I/data/cmsbld/jenkins/workspace/build-any-ib/w/el8_amd64_gcc12/external/gsl/2.6-5e2ce72ea2977ff21a2344bbb52daf5c/include -I/data/cmsbld/jenkins/workspace/build-any-ib/w/el8_amd64_gcc12/external/libuuid/2.34-27ce4c3579b5b1de2808ea9c4cd8ed29/include -isystem/data/cmsbld/jenkins/workspace/build-any-ib/w/el8_amd64_gcc12/lcg/root/6.30.07-f3322c77db1c59847b28fde88ff7218c/include -isystem/data/cmsbld/jenkins/workspace/build-any-ib/w/el8_amd64_gcc12/external/tbb/v2021.9.0-a7089dd5ec356e9a0bc222e109b15cef/include -I/data/cmsbld/jenkins/workspace/build-any-ib/w/el8_amd64_gcc12/cms/vdt/0.4.3-f094bee80112624813c07f9336e08d7d/include -I/data/cmsbld/jenkins/workspace/build-any-ib/w/el8_amd64_gcc12/external/xerces-c/3.1.3-c7b88eaa36d0408120f3c29826a04bf6/include -I/data/cmsbld/jenkins/workspace/build-any-ib/w/el8_amd64_gcc12/external/xz/5.2.5-6f3f49b07db84e10c9be594a1176c114/include -I/data/cmsbld/jenkins/workspace/build-any-ib/w/el8_amd64_gcc12/external/zlib/1.2.11-1a082fc322b0051b504cc023f21df178/include -I/data/cmsbld/jenkins/workspace/build-any-ib/w/el8_amd64_gcc12/external/eigen/3bb6a48d8c171cf20b5f8e48bfb4e424fbd4f79e-3ca740c03e68b1a067f3ed0679234a78/include/eigen3 -I/data/cmsbld/jenkins/workspace/build-any-ib/w/el8_amd64_gcc12/external/fmt/8.0.1-258b4791803c34b7e98cf43693e54d87/include -I/data/cmsbld/jenkins/workspace/build-any-ib/w/el8_amd64_gcc12/external/md5/1.0.0-5b594b264e04ae51e893b1d69a797ec6/include -I/data/cmsbld/jenkins/workspace/build-any-ib/w/el8_amd64_gcc12/external/OpenBLAS/0.3.15-c877ab57fa7b04ce290093588c6c5717/include -I/data/cmsbld/jenkins/workspace/build-any-ib/w/el8_amd64_gcc12/external/tinyxml2/6.2.0-88fe0ec301baf763fa3c485e5b67ed91/include -O2 -pthread -pipe -Werror=main -Werror=pointer-arith -Werror=overlength-strings -Wno-vla -Werror=overflow -std=c++17 -ftree-vectorize -Werror=array-bounds -Werror=format-contains-nul -Werror=type-limits -fvisibility-inlines-hidden -fno-math-errno --param vect-max-version-for-alias-checks=50 -Xassembler --compress-debug-sections -Wno-error=array-bounds -Warray-bounds -fuse-ld=bfd -march=x86-64-v2 -felide-constructors -fmessage-length=0 -Wall -Wno-non-template-friend -Wno-long-long -Wreturn-type -Wextra -Wpessimizing-move -Wclass-memaccess -Wno-cast-function-type -Wno-unused-but-set-parameter -Wno-ignored-qualifiers -Wno-unused-parameter -Wunused -Wparentheses -Werror=return-type -Werror=missing-braces -Werror=unused-value -Werror=unused-label -Werror=address -Werror=format -Werror=sign-compare -Werror=write-strings -Werror=delete-non-virtual-dtor -Werror=strict-aliasing -Werror=narrowing -Werror=unused-but-set-variable -Werror=reorder -Werror=unused-variable -Werror=conversion-null -Werror=return-local-addr -Wnon-virtual-dtor -Werror=switch -fdiagnostics-show-option -Wno-unused-local-typedefs -Wno-attributes -Wno-psabi -DEIGEN_DONT_PARALLELIZE -DEIGEN_MAX_ALIGN_BYTES=64 -Wno-error=unused-variable -DBOOST_DISABLE_ASSERTS -g -O3 -DEDM_ML_DEBUG -flto=auto -fipa-icf -flto-odr-type-merging -fno-fat-lto-objects -Wodr -fPIC -MMD -MF tmp/el8_amd64_gcc12/src/Geometry/TrackerGeometryBuilder/src/GeometryTrackerGeometryBuilder/trackerHierarchy.cc.d src/Geometry/TrackerGeometryBuilder/src/trackerHierarchy.cc -o tmp/el8_amd64_gcc12/src/Geometry/TrackerGeometryBuilder/src/GeometryTrackerGeometryBuilder/trackerHierarchy.cc.o
src/Geometry/TrackerGeometryBuilder/src/RectangularPixelPhase2Topology.cc: In member function 'virtual std::pair<float, float> RectangularPixelPhase2Topology::pixel(const LocalPoint&) const':
  src/Geometry/TrackerGeometryBuilder/src/RectangularPixelPhase2Topology.cc:86:104: error: 'numROC' was not declared in this scope
    86 |                                                << iybin << " " << fractionY << " " << iybin0 << " " << numROC;
      |                                                                                                        ^~~~~~
  src/Geometry/TrackerGeometryBuilder/src/RectangularPixelPhase2Topology.cc:95:104: error: 'numROC' was not declared in this scope
    95 |                                                << iybin << " " << fractionY << " " << iybin0 << " " << numROC;
      |                                                                                                        ^~~~~~
  src/Geometry/TrackerGeometryBuilder/src/RectangularPixelPhase2Topology.cc:133:16: error: 'm_ROW_PER_ROC' was not declared in this scope; did you mean 'm_ROWS_PER_ROC'?
   133 |   if (ixbin0 > m_ROW_PER_ROC || ixbin0 < 0)  //  ixbin < 0 outside range
      |                ^~~~~~~~~~~~~
      |                m_ROWS_PER_ROC
  src/Geometry/TrackerGeometryBuilder/src/RectangularPixelPhase2Topology.cc:143:30: error: 'm_ROW_PER_ROC' was not declared in this scope; did you mean 'm_ROWS_PER_ROC'?
   143 |   if (mpX < 0. || mpX >= 2 * m_ROW_PER_ROC) {
      |                              ^~~~~~~~~~~~~
      |                              m_ROWS_PER_ROC
  gmake: *** [tmp/el8_amd64_gcc12/src/Geometry/TrackerGeometryBuilder/src/GeometryTrackerGeometryBuilder/RectangularPixelPhase2Topology.cc.o] Error 1

@mmusich
Copy link
Contributor

mmusich commented May 24, 2024

DBG builds are broken, see for instance https://cmssdt.cern.ch/SDT/cgi-bin/buildlogs/el8_amd64_gcc12/CMSSW_14_1_DBG_X_2024-05-23-2300/Geometry/TrackerGeometryBuilder

#45038 ?

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

Successfully merging this pull request may close these issues.