-
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
Updated pixel track efficiency codes for phase 1 detector #23160
Conversation
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-23160/4582 |
A new Pull Request was created by @chenxvan for master. It involves the following packages: DQM/SiPixelPhase1Track The following packages do not have a category, yet: DQM/SiPixelPhase1Track @cmsbuild can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
Submitted a PR for the category: |
assign dqm |
New categories assigned: dqm @jfernan2,@vazzolini,@vanbesien,@kmaeshima,@dmitrijus you have been requested to review this Pull request/Issue and eventually sign? Thanks |
Hi @chenxvan , please edit your title : "Updated code for phase 1 detector" could mean that you updated the local reco (for example, since I don't know which code you updated) for hcal (which is also a phase1 detector ...) , and for which purpose ? please be more explicit in the title , thank you. |
Hi @boudoul, I update the title. Please let me know if you need more info. Thanks |
dear @jfernan2, @vazzolini, @vanbesien, @kmaeshima, @dmitrijus , could you please consider this PR for review ? |
please test |
The tests are being triggered in jenkins. |
Comparison job queued. |
The tests are being triggered in jenkins. |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
… On May 17, 2018, at 1:31 PM, Fabio Cossutti ***@***.***> wrote:
@chenxvan @fioriNTU please check in the link provided above the differences for 140.53, i.e.
https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_10_2_X_2018-05-16-2300+23160/26539/140.53_RunHI2011+RunHI2011+RECOHID11+HARVESTDHI/
one should at least ensure that just adding this PR only there is no change there
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
@fabiocos checking the diff link you sent it looks like the PV plots perfectly match on top of each other, but they are flagged as "failing comparison" please check here: For what concern this PR, it doesn't touch any reconstruction module, it only change the way the Layer 1 efficiency for Pixels is computed in DQM, it neither changes the total number of bins in DQM histograms. I would expect only a slight increase in processing time, but nothing can happen outside the Phase1 DQM. |
The differences indeed look numerical-only. If they are produced by fits, then the underlying cause may be the same as discussed in #23105. |
@davidlange6 @makortel yes, this is why I was suspicious, normally the problem of non reproducibility is seen in the Tracking and HLT folders, in this case the pattern of discrepancies is different. I have checked on the same machine rerunning manually the wf 140.53 with and without only this PR merged. Indeed the observed difference looks an artefact of the comparison done by the bot as far as I can see, as I manage to get agreement for 140.53 with the bot compare_using_files.py sequence
so provided there is no further comment and DQM signs I would say this PR could move forward |
@dmitrijus @jfernan2 this PR was already signed by Dmitrijus and the change since then looks trivial, I merge it |
+1 |
merge |
Hi @chenxvan , could you please backport asap ? |
Added offline cuts for all the pixel detector. Added fiducial cuts and backpropagation for Layer 1 detector.