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

MTD, add new geometry scenario (D33 and D34) for bars along z coordinate #24854

Merged
merged 7 commits into from Nov 10, 2018

Conversation

fabiocos
Copy link
Contributor

@fabiocos fabiocos commented Oct 12, 2018

A new MTD geometry scenario (D33) is implemented, with bars (as in D25) but oriented along the z coordinate, instead of phi. RelVals are added in PyReleaseValidation, and the BTLDetId format is updated to accomodate navigation also according to this scenario. This version has a flat geometry, and the passive material layout is updated compared to D24, with the Al support plate in between the crystals and the tracker, and the PCB boards behind. A gap of 4.5 mm is provided between the crystals in z (to accomodate SiPM), crystals are 56 mm long, for a total of 42 modules. The crystal thickness changes in blocks of 14 modules.
Scenario D34 is the same as D33 but without gap between crystals in z, for efficiency studies.

The numbering scheme has been verified with the available test code, and appears correct. The radial crystal position is unmodified wrt the old passive material layout. No overlap is detected, and test workflows for SingleMuon that are added pass correctly.

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @fabiocos (Fabio Cossutti) for master.

It involves the following packages:

Configuration/Geometry
Configuration/PyReleaseValidation
Configuration/StandardSequences
DataFormats/ForwardDetId
Geometry/CMSCommonData
Geometry/MTDCommonData

@cmsbuild, @prebello, @Dr15Jones, @cvuosalo, @civanch, @ianna, @mdhildreth, @pgunnell, @franzoni, @kpedro88, @zhenhu, @fabiocos, @davidlange6 can you please review it and eventually sign? Thanks.
@makortel, @felicepantaleo, @GiacomoSguazzoni, @rovere, @VinInn, @Martin-Grunewald, @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

@fabiocos
Copy link
Contributor Author

@lgray @pmeridian FYI

@fabiocos
Copy link
Contributor Author

please test workflow 22434.0

not testing the new scenario (26607.0) but at least the integrity of existing MTD workflows

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 12, 2018

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/31009/console Started: 2018/10/12 18:58

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-24854/31009/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 32
  • DQMHistoTests: Total histograms compared: 2995443
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2995245
  • DQMHistoTests: Total skipped: 197
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 31 files compared)
  • Checked 134 log files, 14 edm output root files, 32 DQM output files

@kpedro88
Copy link
Contributor

@fabiocos please update Configuration/Geometry/README.md with the new subdetector and detector versions

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 8, 2018

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-24854/31525/summary.html

@slava77 comparisons for the following workflows were not done due to missing matrix map:

  • /build/cmsbld/jenkins/workspace/compare-root-files-short-matrix/results/JR-comparison/PR-24854/7.3_CosmicsSPLoose_UP18+CosmicsSPLoose_UP18+DIGICOS_UP18+RECOCOS_UP18+ALCACOS_UP18+HARVESTCOS_UP18

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 32
  • DQMHistoTests: Total histograms compared: 3013827
  • DQMHistoTests: Total failures: 2
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3013628
  • DQMHistoTests: Total skipped: 197
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 31 files compared)
  • Checked 134 log files, 14 edm output root files, 32 DQM output files

@fabiocos
Copy link
Contributor Author

fabiocos commented Nov 9, 2018

@civanch @ianna @cvuosalo @kpedro88 do you have any comment? I would keep separate this PR and the issue of how to combine with alternative HGCal and/or Tracker setups

@ianna I want to clean the test code in Geometry/MTDCommonData to move it to filtered views, but I will do it in a second moment, as this is now urgent for TDR studies

@kpedro88
Copy link
Contributor

kpedro88 commented Nov 9, 2018

+upgrade

@fabiocos
Copy link
Contributor Author

fabiocos commented Nov 9, 2018

+operations

@fabiocos
Copy link
Contributor Author

fabiocos commented Nov 9, 2018

@civanch @ianna @cvuosalo could you please comment or sign it? We need to have it in 10_4_0_pre2 for tests for the TDR production

@prebello @zhenhu could you please sign it? In any case I will consider ok your previous signature as nothing has really changed in PyReleaseValidation

@zhenhu
Copy link
Contributor

zhenhu commented Nov 9, 2018

+1

@kpedro88
Copy link
Contributor

kpedro88 commented Nov 9, 2018

@fabiocos @lgray there are now 4 active versions for the timing detector geometry. Which one should I combine with the new HGCal to make a TDR candidate workflow?

@fabiocos
Copy link
Contributor Author

@kpedro88 in my understanding the baseline choice is D33, but I let @pmeridian and @lgray to comment

@pmeridian
Copy link
Contributor

Tempatively we could go with D33. MTD management has not yet taken a final choice (we have brought this up at last steering group, we will re-iterate next Tuesday)

@civanch
Copy link
Contributor

civanch commented Nov 10, 2018

+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 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)

@fabiocos
Copy link
Contributor Author

+1

@cmsbuild cmsbuild merged commit 331d645 into cms-sw:master Nov 10, 2018
@fabiocos fabiocos deleted the fc-MTDbarz branch December 20, 2018 16:50
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