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

Scenario2026D49: no Overlaps inside the Muon System and all MB4 Shiel… #27866

Merged
merged 8 commits into from Sep 18, 2019

Conversation

slomeo
Copy link
Contributor

@slomeo slomeo commented Aug 25, 2019

PR description:

Scenario 2026 (D49): No overlaps inside the Muon System, all MB4 Shields are present, fixed Hidden Volumes for DD4HEP migration

PR validation:

see picture from cmsShow
Screenshot 2019-08-25 16 54 45

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-27866/11616

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @slomeo (Sergio Lo Meo) for master.

It involves the following packages:

Configuration/Geometry
Configuration/StandardSequences
Geometry/CMSCommonData
Geometry/MuonCommonData

@civanch, @Dr15Jones, @cvuosalo, @ianna, @mdhildreth, @cmsbuild, @franzoni, @kpedro88, @fabiocos, @davidlange6 can you please review it and eventually sign? Thanks.
@vargasa, @makortel, @felicepantaleo, @GiacomoSguazzoni, @rovere, @VinInn, @Martin-Grunewald, @ptcox, @dgulhan this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

@@ -878,7 +865,7 @@
("O2","T14","C6","M3","F3","I7") : "D44",
("O3","T15","C8","M3","F2","I10") : "D45",
("O3","T15","C9","M3","F2","I10") : "D46",
("O3","T15","C10","M3","F3","I10") : "D47",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@slomeo - this doesn't look right. I think you need to rebase your branch.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ianna : Yes, you are right. I restored the files dict2026Geometry.py and GeometryConf.py present in CMSSW_11_0_X_2019-08-23-2300 (containing D47) and I added the new lines for D49. I have created a new cmsExtended2026D49_cfi.py and all tests (geometry and overlaps) are fine.

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-27866/11618

@cmsbuild
Copy link
Contributor

Pull request #27866 was updated. @civanch, @Dr15Jones, @cvuosalo, @ianna, @mdhildreth, @cmsbuild, @franzoni, @kpedro88, @fabiocos, @davidlange6 can you please check and sign again.

@cvuosalo
Copy link
Contributor

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 26, 2019

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/2188/console Started: 2019/08/26 17:54

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@kpedro88
Copy link
Contributor

kpedro88 commented Sep 9, 2019

+upgrade

@fabiocos
Copy link
Contributor

fabiocos commented Sep 9, 2019

@kpedro88 this looks just a variant of D46 with muon overlaps fixed, @slomeo do you confirm? My question is: do we need the latter, if we integrate the former?

@fabiocos
Copy link
Contributor

fabiocos commented Sep 9, 2019

+operations

the update to central configurations looks coherent with the purpose of the PR

@fabiocos
Copy link
Contributor

fabiocos commented Sep 9, 2019

@ianna @cvuosalo any objection or further suggestion?

@slomeo
Copy link
Contributor Author

slomeo commented Sep 9, 2019

@slomeo do you confirm?
@fabiocos Yes, I do. it is D46 with 03 and M3 replaced with 04 and M4 (no overlaps in the Muon System)

@kpedro88
Copy link
Contributor

kpedro88 commented Sep 9, 2019

@fabiocos for now we need to keep M3 because it was used in the latest TDR production. In the future, we can deprecate M3 in favor of M4.

@fabiocos
Copy link
Contributor

@cvuosalo @ianna @slomeo is there an issue with this PR that is keeping the geometry signature on hold?

@cvuosalo
Copy link
Contributor

@fabiocos: It looks OK to me. @ianna had objections earlier, so I was waiting to see if she is satisfied. But she will be away tomorrow, so maybe the PR should go forward.

@fabiocos
Copy link
Contributor

@ianna any objection?

@ianna
Copy link
Contributor

ianna commented Sep 16, 2019

before: 79 overlaps and 136 extrusions (pic 1)
after: 78 overlaps and 88 extrusions (pic 2)
Screen Shot 2019-09-16 at 14 48 24
Screen Shot 2019-09-16 at 14 59 45

Most of the overlaps are at around 10 to -6, but there are extrusions of order of cm:

= Overlap ov00000: hgcal:HGCal extruded by: hgcal:HGCal/hgcal:HGCalHEmix_1 ovlp=1.48

@fabiocos - it's your decision if we want all these to be eliminated or keep them as is.

@fabiocos
Copy link
Contributor

@slomeo @ianna @kpedro88 what is the real purpose of this PR? In my understanding it was supposed to provide a scenario do be used for muon studies. If the HGCal scenario used has troubles preventing production, and that part is not crucial for the studies to be done, I wonder whether one should not reconsider which HGCal description has to be used here. Of course I am not a fan of inflating artificially the number of scenarios, but we should also avoid conflicts. To be discussed at the ORP

@slomeo
Copy link
Contributor Author

slomeo commented Sep 17, 2019

@slomeo @ianna @kpedro88 what is the real purpose of this PR? In my understanding it was supposed to provide a scenario do be used for muon studies.

Yes @fabiocos, the purpose of my PR was to provide a 2026 scenario for muon studies (i.e Neutron Background studies for Run4/PhaseII)

@fabiocos
Copy link
Contributor

@ianna @cvuosalo as discussed at the ORP of 2019/09/17 we move forward with this PR so as to provide the scenario with the overlaps in the muon system fixed. The fix for the issues in the calorimetry part will have to be provided in another PR

@fabiocos
Copy link
Contributor

+1

@fabiocos
Copy link
Contributor

merge

@cmsbuild cmsbuild merged commit 8a5a5c9 into cms-sw:master Sep 18, 2019
@cvuosalo
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.

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.

None yet

9 participants