-
Notifications
You must be signed in to change notification settings - Fork 4.6k
MTD Geometry: Update ETL numbering scheme, add corresponding new geometry scenarios #49234
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
Conversation
commit f20ddce Author: Leonardo Lanteri <[email protected]> Date: Tue Oct 28 10:53:47 2025 +0100 additional cleaning and added a method in MTDDigiGeometryAnalyzer.cc commit 8f3142b Author: Leonardo Lanteri <[email protected]> Date: Mon Oct 27 15:09:08 2025 +0100 cleaned the branch, should be ready for a PR commit 145c2c4 Author: Leonardo Lanteri <[email protected]> Date: Tue Oct 21 15:21:10 2025 +0200 etl v12 working, there is still a sanity check on the PPMTDParameters vectors to restore commit c2ea4bc Author: Leonardo Lanteri <[email protected]> Date: Fri Oct 10 14:56:28 2025 +0200 small modification to etl.xml v10 due to a small shift in x found for a module, now v10 and v12 are equivalent and both correct commit 7062166 Author: Leonardo Lanteri <[email protected]> Date: Thu Oct 9 14:48:54 2025 +0200 added the last missing piece to upgradeWorkflowComponents.py commit d2a6b62 Author: Leonardo Lanteri <[email protected]> Date: Thu Oct 9 11:36:27 2025 +0200 almost generated Run4D201 commit c551afa Author: Leonardo Lanteri <[email protected]> Date: Wed Oct 8 17:27:57 2025 +0200 added ETL v12 and introduced the scenario D201 in dictRun4Geometry.py and in the README.md commit c928467 Author: Leonardo Lanteri <[email protected]> Date: Mon Oct 6 15:08:23 2025 +0200 changed default scenario, everything works for vv2 now commit 4f50c6e Author: Leonardo Lanteri <[email protected]> Date: Fri Sep 26 17:04:30 2025 +0200 small modifications, mainly cleaning, NB the Default scenario that was set commit 712188b Author: Fabio Cossutti <[email protected]> Date: Thu Sep 18 15:13:51 2025 +0200 Update orderETLSector to work without module number, update RecoMTD/DetLayers test for topology validation commit 68570f0 Author: Fabio Cossutti <[email protected]> Date: Wed Sep 17 15:22:52 2025 +0200 Update verbosity for consistency among tests, update orderETLSector, fix version usage in topology commit 11df807 Author: Fabio Cossutti <[email protected]> Date: Tue Sep 16 11:35:59 2025 +0200 Add workflows for D200, update service hybrid methods, debugging statements (to be removed) commit 50ab8aa Author: Fabio Cossutti <[email protected]> Date: Fri Sep 12 15:03:13 2025 +0200 Add ETL v11, adapt code, clean existing implementation commit aa189c8 Author: Fabio Cossutti <[email protected]> Date: Wed Sep 10 17:57:29 2025 +0200 Updates for scenario v11 commit e265364 Author: Fabio Cossutti <[email protected]> Date: Wed Sep 10 17:39:30 2025 +0200 Add a ETL v11/scenario D200, for now equal to v9 Co-authored-by: Leonardo Lanteri <[email protected]>
|
cms-bot internal usage |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-49234/46581
|
|
A new Pull Request was created by @leonardolanteri for master. It involves the following packages:
@AdrianoDee, @DickyChant, @Dr15Jones, @antoniovagnerini, @bsunanda, @civanch, @cmsbuild, @davidlange6, @fabiocos, @ftenchini, @jfernan2, @kpedro88, @makortel, @mandrenguyen, @mdhildreth, @miquork, @srimanob can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
please test workflow 35634.0,36034.0 with cms-data/Geometry-TestReference#22 |
|
type mtd |
| } | ||
|
|
||
| MTDTopology::ETLfaceLayout tmpFace; | ||
| std::vector<int> dummy{{0}}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@leonardolanteri the CLANG issue looks here, please fix it. The size of dummy in these situations is 1, right? So it should be enough
std::vector<int> dummy(1,0);
|
+1 Size: This PR adds an extra 16KB to repository The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: You can see more details here: Comparison SummarySummary:
|
|
+1 |
|
+simulation |
|
@leonardolanteri , @fabiocos , is it really needed and for how long both I21 and I22? Which one is expected to be the baseline? |
|
@civanch as we presented recently, the two scenarios have been used to assess the physics impact of a descoping or staging of the project, so yes, they have been and are needed at least until when a final decision is taken, likely in some moment end of winter. And whether one of them will be the final one, or we will need a further adjustment is still not completely defined. Software cannot be more final than detector engineering is. These scenarios are effectively replacing I19 and I20, never really used so far, by 1) updating the geometry adding an extra logical layer, which is used for 2) updating in a backward incompatible way the ETL numbering scheme and address the needs of the alignment studies. |
|
We might say that we do not really need any more I19 and I20, after this is integrated... |
|
+pdmv |
|
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 now be reviewed by the release team before it's merged. @ftenchini, @sextonkennedy, @mandrenguyen (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
PR description:
Updated the ETL numbering scheme by defining a new ETLDetId to have a direct correlation with the ETL Service Hybrids.
Added corresponding new ETL geometry scenarios (I21 and I22)
The new ID will be described in a CMS Detector note under preparation and is documented in the presentation:
https://indico.cern.ch/event/1527057/contributions/6426567/attachments/3036416/5362380/Update_ETL_DETid.pdf
PR validation:
The code compiles, runs and has been extensively checked. Unit tests run smoothly with the updated references.