Shift to orbit frame #35679
Shift to orbit frame #35679
Conversation
…ion of the beam spot)
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35679/25969
|
|
A new Pull Request was created by @mundim for master. It involves the following packages:
@cmsbuild, @civanch, @mdhildreth can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
Just to let you now: @clemencia, @fabferro |
| @@ -144,26 +143,30 @@ | |||
| empiricalAperture56_xi0_int = cms.double(0.074), | |||
| empiricalAperture56_xi0_slp = cms.double(6.604E-04), | |||
| empiricalAperture56_a_int = cms.double(-22.7), | |||
| empiricalAperture56_a_slp = cms.double(1.600) | |||
| empiricalAperture56_a_slp = cms.double(1.600), | |||
There was a problem hiding this comment.
It was left behind after a change in the config. I can remove it for sure if this is a problem. Shouldn't it be detected by code-checks and/or code-format option?
SimTransport/PPSProtonTransport/python/OpticalFunctionsConfig_cfi.py
Outdated
Show resolved
Hide resolved
…onsumes in HectorTransport.cc
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35679/25996
|
|
Pull request #35679 was updated. @cmsbuild, @civanch, @mdhildreth can you please check and sign again. |
|
please test |
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4242cb/19703/summary.html Comparison SummarySummary:
|
|
+1 |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35679/26058
|
|
Pull request #35679 was updated. @cmsbuild, @civanch, @mdhildreth can you please check and sign again. |
|
please test |
|
Thank you @mundim #include "Utilities/PPS/interface/PPSUtilities.h"is still missing from SimTransport/PPSProtonTransport/src/OpticalFunctionsTransport.cc |
That's weird, the compilation went fine in my working area. Let me check...Sorry about it. |
It is included in which is a base class from which OpticalFunctionsTransport is derived. |
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4242cb/19738/summary.html Comparison SummarySummary:
|
|
@cms-sw/simulation-l2 I imagine that you have nothing against the last (rather trivial) updates applied after your previous signature. I'm going to merge this for the next IB, please comment in case you see any issue with it |
|
+1 |
cmsbuild
added
fully-signed
simulation-approved
and removed
simulation-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. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
| m_beamline45(nullptr), | ||
| m_beamline56(nullptr), | ||
| beamParametersToken_(iC.esConsumes()), | ||
| beamspotToken_(iC.esConsumes()) { |
There was a problem hiding this comment.
thanks to this, now the SIM step depends on the reconstruction level beamspot... which implies the whole chain of MC conditions production now folds because you need the reconstruction beamspot to even produce the PU library. Nice!
@francescobrivio
There was a problem hiding this comment.
this should use the generator level smearing in my opinion @civanch
PR description:
This PR implements a shift of the starting position of the scattered proton to the so called "orbit" reference frame, the one that defines the beam spot at the IP and in which the optical function for the proton propagation is defined.
No change in the output is expected unless a better matching between the generated and reconstructed proton kinematics.
PR validation:
All tests ran without any issues. In particular, runTheMatrix was run with the standard wf and also the ones specific for pps:
runTheMatrix.py -l limited,11725.0,11925.0 --ibeos -j8
with output:
42 41 40 31 20 4 1 1 1 tests passed, 0 0 0 0 0 0 0 0 0 failed