changing strips from 192 to 96 #25701
Conversation
|
The code-checks are being triggered in jenkins. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-25701/8079
|
|
A new Pull Request was created by @bapavlov for master. It involves the following packages: Geometry/RPCGeometryBuilder @civanch, @Dr15Jones, @cvuosalo, @ianna, @mdhildreth, @cmsbuild can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
please test |
|
The tests are being triggered in jenkins. |
|
what backwards compatibility would this request break?
… On Jan 18, 2019, at 2:25 PM, bapavlov ***@***.***> wrote:
Changing the number of iRPC strips from 192 to 96. The number 192 has been used for studies, while 96 is the number included in the TDR (page 165 , table 5.2) : https://cds.cern.ch/record/2283189/files/CMS-TDR-016.pdf
You can view, comment on, or merge this pull request online at:
#25701
Commit Summary
• changing strips from 192 to 96
File Changes
• M Geometry/RPCGeometryBuilder/data/PhaseII/RPCSpecs.xml (4)
Patch Links:
• https://github.com/cms-sw/cmssw/pull/25701.patch
• https://github.com/cms-sw/cmssw/pull/25701.diff
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
I do not expect backward compatibility issues. This value is for the future chmabers only. During the detailed studies done for the Upgrade TDR we have tried with different number of strips (32, 64, 96 and 192) and all the changes are transparent fro the RECO step. Hardware experts have fixed the number of strips to 96 and this is the final value in the TDR. Now some trigger studies are under way and the RPC trigger experts would like to have 96 as a default value, because the iRPCs are expected to be with 96 strips. |
|
Comparison job queued. |
|
Comparison is ready Comparison Summary:
|
|
@bapavlov: Are the differences in the comparison tests expected and understood? |
|
@cvuosalo - FYI, CMS policy is to keep the XML files versioned. We have agreed to keep different versions of the files in the structured directories |
Looking on the comparisons I do not see significant changes - for instance in samples with muons in final state, where RPCs are expected to contribute. For instance Z->mumu https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_10_5_X_2019-01-18-1100+25701/30113/1330.0_ZMM_13+ZMM_13+DIGIUP15+RECOUP15_L1TMuDQM+HARVESTUP15_L1TMuDQM/ The changes in "DQMHistoSizes: changed ( 21234.0,... ): -270.000 KiB RPC/RPCEfficiency". It's OK that the strip efficiency histogram are reduced in size by reducing the number of strips. |
Well, I missed somehow the convention. Should I make changes ? |
|
Comparison job queued. |
|
Comparison is ready Comparison Summary:
|
|
assign upgrade as we discuss of upgrade geometry |
|
New categories assigned: upgrade @kpedro88 you have been requested to review this Pull request/Issue and eventually sign? Thanks |
|
@bapavlov as a general policy we should not modify older geometry versions, but provide new scenarios where the update is applied. The file updated here has now a reasonable name. In case you might integrate the PR adding a geometry scenario where it is effectively used, if you want to practically test it in a production workflow. I let @kpedro88 comment further |
|
To follow the policy, we should add a muon detector version M3 with this file and then make a workflow that incorporates it. Currently we have a large proliferation of Phase 2 workflows that are using M2 (with the apparently old number of strips), see https://github.com/cms-sw/cmssw/tree/master/Configuration/Geometry. I don't really think it is feasible to update all of them to new versions; some of them have been used in production. But we can build on this change for subsequent Phase 2 geometry updates. In the list from #25701 (comment), I'm surprised to see that Run 2 workflows are using the PhaseII RPC spec file. Is this intentional? |
OK. It sounds good.
Well, I'm also surprised ...I just search github repository for string "PhaseII/RPCSpecs.xml" and contrary to my initial expectations a lot of files seems to use it. |
|
@bapavlov Is this file RPCSpecs the only file which changes M2 to M3. We are in the process of making a new scenario D41. We plan to include this in that case. |
|
+1 |
cmsbuild
added
fully-signed
geometry-approved
and removed
geometry-pending
pending-signatures
labels
|
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. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 this PR in itself does not change any workflow, but will be used to define a new one (D41) |
Changing the number of iRPC strips from 192 to 96. The number 192 has been used for studies, while 96 is the number included in the TDR (page 165 , table 5.2) : https://cds.cern.ch/record/2283189/files/CMS-TDR-016.pdf