Migrate away from explicit comparisons of GeomDetEnumerators::PixelBarrel/PixelEndcap #11624
Conversation
Now I can safely assume in production code that each SubDetector is either barrel or endcap.
…s instead of explicit comparisons of enumerators These changes are needed to suppor Phase1+ upgrades, where different values of GeomDetEnumerators enumeration are used instead of the Run1/2 values.
…plicit tracker GeomDetEnumerators
makortel
changed the title
|
A new Pull Request was created by @makortel (Matti Kortelainen) for CMSSW_7_6_X. Migrate away from explicit comparisons of GeomDetEnumerators::PixelBarrel/PixelEndcap It involves the following packages: CalibTracker/SiPixelESProducers @civanch, @diguida, @lveldere, @cvuosalo, @ianna, @mdhildreth, @cmsbuild, @ssekmen, @franzoni, @Dr15Jones, @cerminar, @slava77, @mmusich can you please review it and eventually sign? Thanks. |
|
please test |
|
The tests are being triggered in jenkins. |
|
+1 |
|
+1 |
|
+1 Converting detector comparisons of the pixel detector to support Phase 1 changes. There should be no change in the logic, only the method of comparison. There should be no change in run-time behavior. The code changes are satisfactory, and Jenkins tests against baseline CMSSW_7_6_X_2015-10-05-2300 show no significant differences, as expected. |
|
+1 |
|
+1 |
|
+1 |
cmsbuild
added
alca-approved
fully-signed
and removed
alca-pending
pending-signatures
labels
|
This pull request is fully signed and it will be integrated in one of the next CMSSW_7_6_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @Degano, @smuzaffar |
|
+1 |
Migrate away from explicit comparisons of GeomDetEnumerators::PixelBarrel/PixelEndcap
|
Quoting Vincenzo from the private thread
So probably the damage is minimal (or even non-existent?). IIRC in the typical workflows (like in PR tests) this branch of the code is not run. |
|
Indeed, validating dead code is tough |
This PR migrates all code (found with
git grep GeomDetEnumerators::Pixel) from usingGeomDetEnumerators::PixelBarrel/PixelEndcapto either use the helper functions in GeomDetEnumerators, orGeomDetEnumerators::subDetGeom(one case) (one case was also fromsubDetGeomto the helper functions). These changes are needed for the Phase1/2 upgrades, where pixel barrel hasP1PXBand pixel endcapP1PXECorP2PXECvalues. (Something similar is probably needed for Phase2 strips, but that is left for future work).I also added a simple unit test for GeomDetEnumerators to ensure that all subdets are either
isBarrel()orisEndcap(), so that I can safely assume in production code that, according to these functions, if something is not barrel, it is endcap (and vice versa). (Mostly because the condition that spurred this PR wasif(pixel barrel) ... else if(pixel endcap) ... else throw).Together with #11621 and #11609, the reco step of phase1 workflow
runTheMatrix.py --what upgrade -l 10000technically works. (The final HARVESTING step fails on missing input file, because DQM/VALIDATION are not part of the reco step)Tested in 760pre6, no changes expected in Run1/2 workflows.
@rovere @VinInn @boudoul @venturia @mark-grimes