Integrating PPS full sim in Run3 (only) #30575
Conversation
|
The code-checks are being triggered in jenkins. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-30575/16810
|
|
A new Pull Request was created by @mundim for master. It involves the following packages: Configuration/Eras @perrotta, @smuzaffar, @civanch, @Dr15Jones, @makortel, @emeschi, @mdhildreth, @cmsbuild, @mommsen, @franzoni, @silviodonato, @slava77, @jpata, @fabiocos, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
| @@ -575,6 +575,23 @@ | |||
| run2_common.toModify( g4SimHits.HFShowerLibrary, FileName = 'SimG4CMS/Calo/data/HFShowerLibrary_npmt_noatt_eta4_16en_v4.root' ) | |||
| run2_common.toModify( g4SimHits.HFShower, ProbMax = 0.5) | |||
|
|
|||
|
|
|||
| from Configuration.Eras.Modifier_ctpps_2021_cff import ctpps_2021 | |||
There was a problem hiding this comment.
@mundim , if the modifier is configured in this way, then central detector simulation is disabled and only PPS simulation is enabled. In that case, this modifier should be applied only for specific ctpps_2021 simulation. I assume it is the case and in the normal WF used in tests this should not be used.
There was a problem hiding this comment.
Hi @civanch (sorry for the late reply, my laptop had to be taken to the maintenance, I'm using a tablet).
I'm not sure I understood the problem. I just picked up from the comments in the PR 29916, as suggested by @makortel for Run2 (#29916 (comment)) and replaced ctpps_2016 by ctpps_2021.
Do I have to make others changes or the rebase as your next message is enough?
There was a problem hiding this comment.
@civanch Could you clarify in which way the customization below disables central detector simulation? Is it perhaps the g4simhits.Generator.HepMCProductLabel = 'LHCTransport?
There was a problem hiding this comment.
I don' t believe so. We have done private pps simulation with this label and the central CMS simulation was not disabled.
|
@mundim , rebase is needed. |
|
The code-checks are being triggered in jenkins. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-30575/16921
|
|
Pull request #30575 was updated. @perrotta, @smuzaffar, @civanch, @Dr15Jones, @makortel, @emeschi, @mdhildreth, @cmsbuild, @mommsen, @franzoni, @silviodonato, @slava77, @jpata, @fabiocos, @davidlange6 can you please check and sign again. |
|
please test |
|
The tests are being triggered in jenkins.
|
|
+1 |
|
+1
|
|
@mundim the static analyzer shows that It could be cleaned either here, or in the next PR, if you have to touch the same file |
If you don't mind, I prefer to leave it to the next PR (which will be very small, I mean, very few changes). This one has already a huge history, any change will restart it again. What do you think? And still, it is planned to change the transport again soon, when realistic optics will be available for run3, so we will have at least two opportunities to address this problem. |
Sure, I only wanted to point it to you, in case you missed it! |
thanks... I'll do it right away and leave it in git for the proper time to submit. I'll need to scan all PR's because there are other issues that were left behind for "future PR". As soon as we get this integrated, it will be time to address them all. |
|
@makortel , @Dr15Jones , @silviodonato , @qliphy , please check and sign this PR - it stay too long before can be merged, now is in a good shape and include needed modifications for PPS simulation. Following PR will be needed, when GT with full Run-3 geometry will be available. |
|
+operations |
|
Kind reminder @cms-sw/core-l2 |
| @@ -19,7 +19,7 @@ | |||
| ), | |||
| LHCTransport = cms.PSet( | |||
| initialSeed = cms.untracked.uint32(87654321), | |||
| engineName = FastSimEngine | |||
| engineName = cms.untracked.string('TRandom3') | |||
There was a problem hiding this comment.
Do we really want to use TRandom3 engine? @civanch
|
@makortel , initially it was told that Hector transport may be done only with TRandom3. I do not think that migration is impossible. Migration from hard-coded TRandom3 to a general scheme of CMSSW should be done in a separate PR, dedicated only for that. Let us sign this and making an issue. |
|
+1 Thanks @civanch for the explanation. |
cmsbuild
added
core-approved
fully-signed
and removed
core-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. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
PR description:
Another step towards the ntegration of PPS full simulation into cmssw (still only for Run3).
This PR propagates the changes made by @civanch in the central simulation. Now, PPS simulation is triggered by the flag LHCTransport (False by default), which needs to be set to TRUE. Since the PPS geometry is not yet loaded into the DB, the digi was removed from the chain in order to avoid failure in the standard tests. In summary, this PR changes SimPPS/PPSSimTrackProducer and PPSProtonTransport to save inthe LHCTransport data container only the propagate protons (only secondary vertices in from of PPS region).
Another PR will be issued when PPS geometry gets loaded into the DB, pluging in the digi part.
PR validation:
scram b runtests ran with no issue
scram b code-checks and scram b code-formats also ran ok
runTheMatrix.py -l limited -i all --ibeos was ran again with the results:
35 33 27 20 15 4 1 1 1 tests passed, 0 1 5 0 0 0 0 0 0 failed
All the 6 wf failed due to timeout. Running them again gave:
6 6 6 5 2 tests passed, 0 0 0 0 0 failed
if this PR is a backport please specify the original PR and why you need to backport that PR:
Before submitting your pull requests, make sure you followed this checklist: