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

Bugfix: Remove TotemT2 Envelope Volume #24563

Merged

Conversation

ianna
Copy link
Contributor

@ianna ianna commented Sep 17, 2018

  • Add a forward:TotemT2 missing volume which is used in other files: ionpump.xml and castor.xml

@ianna
Copy link
Contributor Author

ianna commented Sep 17, 2018

please test

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 17, 2018

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/30458/console Started: 2018/09/17 16:10

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @ianna (Ianna Osborne) for master.

It involves the following packages:

Geometry/CMSCommonData

@civanch, @Dr15Jones, @cvuosalo, @ianna, @mdhildreth, @cmsbuild, @kpedro88 can you please review it and eventually sign? Thanks.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

@ianna
Copy link
Contributor Author

ianna commented Sep 17, 2018

+1

The scenario is orthogonal to the production workflows and is not used in the relval tests.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@civanch
Copy link
Contributor

civanch commented Sep 17, 2018

@ianna , Totem2 has been removed for 2018 run physically, Castor not yet installed, it will be installed in November. It was discussed in the beginning of the year, that Totem2 is taken out but Castor geometry will be left, because Castor cannot affect central detector. If I understand correctly the description of this PR, then it would be better to close this PR.

@cmsbuild
Copy link
Contributor

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

Comparison Summary:

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

@ianna
Copy link
Contributor Author

ianna commented Sep 18, 2018

@civanch - FYI, the included file does not define Totem, but the forward region. This scenario is (and must be) identical to 2018 with the exception of the detailed cavern and MB4 shields. It is needed for production asap which has been discussed and agreed at the PPD meeting.

@cmsbuild
Copy link
Contributor

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

@ianna
Copy link
Contributor Author

ianna commented Sep 26, 2018

The latest commits added to remove the following warnings related to an incorrect definition of the Totem2 envelope volume:

WARNING: MCParticlePairFilter : size of some vectors not matching with 2!!
Cannot create Polycone with rInner > rOuter for the same Z
         rInner > rOuter for the same Z !
         rMin[2] = 3200 -- rMax[2] = 250Cannot create a Polycone with no 
contiguous segments.
                 Segments are not contiguous !
                 rMin[1] = 42 -- rMax[2] = 250
                 rMin[2] = 3200 -- rMax[1] = 250%MSG-w 
SimG4CoreApplication:  (NoModuleName) 19-Sep-2018 13:48:09 CEST pre-events

-------- WWWW ------- G4Exception-START -------- WWWW -------
*** G4Exception : GeomMgt0001
       issued by : G4UPolycone::BoundingLimits()
Inconsistency in bounding boxes for solid: TotemT2 !
BBox min: wrapper = (-3200,-3200,13381) solid = (-250, -250, 13381)
BBox max: wrapper = (3200,3200,15350) solid = (250, 250, 15350)*** This 
is just a warning message. ***
-------- WWWW -------- G4Exception-END --------- WWWW -------

and some other warnings about TotemT2...

@ianna
Copy link
Contributor Author

ianna commented Sep 26, 2018

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 26, 2018

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/30660/console Started: 2018/09/26 17:49

@ianna
Copy link
Contributor Author

ianna commented Sep 26, 2018

+1

The scenario is orthogonal to the production workflows and is not used in the relval tests.

@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-24563/30660/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: 3162160
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3161962
  • 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

+upgrade

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

<Constant name="TotemBeamR5" value="3.575*cm"/>
<Constant name="TotemBeamZ1" value="7.9500*m"/>
<Constant name="TotemBeamZ2" value="13.381*m"/>
<Constant name="TotemBeamZ3" value="13.439*m"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

@ianna @civanch does this and other lines imply that in 10_2_X we are using a wrong geometry description for the Fall18 production?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, the changes are related to a wrongly defined envelope volume. It only produces G4 warnings.

@@ -239,10 +239,10 @@
'Geometry/MuonCommonData/data/mfshield/2017/v1/mfshield.xml',
'Geometry/MuonCommonData/data/gemf/TDR_BaseLine/gemf.xml',
'Geometry/MuonCommonData/data/gem11/2017/v2/gem11.xml',
'Geometry/ForwardCommonData/data/forward/2018/v1/forward.xml',
Copy link
Contributor

Choose a reason for hiding this comment

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

@ianna @civanch the addition of CASTOR should be relevant only for HI data, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. Unfortunately, there are some modules which ask for Castor hits in any case. Given that the production stops when they are not available, the easiest way was to keep it in the geometry description. The alternative would be to customize the producers, but it's not in my domain.

'Geometry/ForwardCommonData/data/forwardshield/2017/v1/forwardshield.xml',
'Geometry/ForwardCommonData/data/brmrotations.xml',
'Geometry/ForwardCommonData/data/PostLS2/brm.xml',
'Geometry/ForwardCommonData/data/ionpump.xml',
Copy link
Contributor

Choose a reason for hiding this comment

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

@ianna @civanch is this material that will be removed now?

@fabiocos
Copy link
Contributor

@ianna although this is for 10_3_X (to be used for HI data taking), could you please clarify the possible implications of this PR for 10_2_X ongoing SIM production? Are we using a wrong geometry configuration there?

@ianna
Copy link
Contributor Author

ianna commented Sep 28, 2018

@fabiocos - If the 10_2_X production is ongoing, there should be no problem. This PR is for an alternative scenario.

@civanch
Copy link
Contributor

civanch commented Sep 28, 2018

@fabiocos , we discuss at spring, that TOTEM1,2 detectors are physically dismantled forever and Castor should be installed in November, so we take away TOTEM from production of 2018 but leave Castor in in order to have uniform simulation for whole year. Physically any particle entering Castor will never reflected back in our simulation, CPU overhead due to Castor is minimal. These were arguments to keep Castor.

@fabiocos
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 688da1d into cms-sw:master Sep 28, 2018
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

5 participants