-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
PPS: bugfix in proton reconstruction. #26739
Conversation
FYI @fabferro |
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-26739/9729
|
A new Pull Request was created by @jan-kaspar for master. It involves the following packages: CalibPPS/ESProducers @perrotta, @slava77, @christopheralanwest, @tocheng, @cmsbuild, @franzoni, @tlampen, @pohsun can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
The tests are being triggered in jenkins. |
Comparison job queued. |
Comparison is ready @slava77 comparisons for the following workflows were not done due to missing matrix map:
Comparison Summary:
|
@tocheng @christopheralanwest @pohsun could you please have a look? |
+1 |
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) |
+1 |
PR description:
This PR fixes two problems observed in rare conditions:
CTPPSInterpolatedOpticalFunctionsESSource
- the aim is to check the freshly-obtained xangle value, not the buffered one.CTPPSProtonProducer
, the re-initialisation is triggered with the help of an ESWatcher. With the recently introduced!hTracks->empty()
condition, not all xangle transitions are seen anymore. Using ESWatcher is better anyway - the decision on updating optics-related data is made just on one place.The fix is important for the UL re-reco. We apologise for the late submission.
PR validation:
Comparison on selected datasets, below, shows no difference wrt. the current master: blue = with this PR, red dashed = before this PR.
![make_cmp](https://user-images.githubusercontent.com/17830858/57527944-3c134b80-7331-11e9-98eb-cac0e85fc71b.png)
The problems addressed in this PR were discovered in a systematic test applied to (almost) all fills in which PPS Roman Pots participated (2016 to 2018). The problems occurred in about 5 out of 400 tests. After the fix all tests pass successfully. For the record, some of the originally failing input files were copied here:
These files are skims from the "official" AOD ntuples.
Finally, AddOnTests were run on LXPLUS, yielding: 46 tests passed, 1 failed. The failing test was running PhysicsTools/PatAlgos/test/IntegrationTest_cfg.py and the failure was due to an XROOT problem.