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
disable ZS mark and pass for 2017 data. it affects pre-TS1 data only. #20115
Conversation
A new Pull Request was created by @deguio (Federico De Guio) for master. It involves the following packages: RecoLocalCalo/HcalRecProducers @perrotta, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
please test |
The tests are being triggered in jenkins. |
from what I understood earlier this should better be implemented as an era dependent flag (better if done in conditions...)
… On Aug 10, 2017, at 4:46 PM, Federico De Guio ***@***.***> wrote:
earlier this year we realized that there was an effective ZS applied at reco level for HF which was coming from the MarkAndPass flag being set in the RAW data. that flag is now unset and the HF is read and reconstructed in full readout for real starting from TS1. However, in order to recover the wanted non-suppressed behaviour in HF for the early 2017 data in the upcoming re-recos, we would like to merge this change.
You can view, comment on, or merge this pull request online at:
#20115
Commit Summary
• disable ZS mark and pass for 2017
File Changes
• M RecoLocalCalo/HcalRecProducers/python/hfprereco_cfi.py (2)
Patch Links:
• https://github.com/cms-sw/cmssw/pull/20115.patch
• https://github.com/cms-sw/cmssw/pull/20115.diff
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
I understood this flag was also not desired for current data taking.
… On Aug 10, 2017, at 4:57 PM, Kevin Pedro ***@***.***> wrote:
hfprereco is only used in 2017 onward, so I don't think the change needs to be isolated by an Era
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
please provide some plots for expected changes in data and MC as a part of this PR description. |
hi, the impact in terms of number of rechits from HF can be seen at: no changes are expected in MC. |
+1 The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: |
Comparison job queued. |
On 8/10/17 9:06 AM, Federico De Guio wrote:
hi,
the change is motivated by the jetMet report at today's PPD. see for
example slide 7 at:
https://indico.cern.ch/event/659010/contributions/2687022/attachments/1506909/2348470/PPD.pdf
for the coming rerecos their wish is to have this fixed for early 2017 data.
the impact in terms of number of rechits from HF can be seen at:
https://goo.gl/FahKEM
the run when the change at p5 happened is: 298653
Thanks.
So, the change is large.
The normal procedure steps are:
- 93X gets merged
- relvals are made and validated for relevant data
- then a backport is accepted and deployed in 92X.
So, the 92X PR will stick around unmerged until then.
…
no changes are expected in MC.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#20115 (comment)>, or
mute the thread
<https://github.com/notifications/unsubscribe-auth/AEdcbjUxB3-ln5UacyuRGCsMk0kIJywtks5sWymygaJpZM4OziFv>.
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pr-code-checks/PR-20115/27 |
Hi, let me just clarify that:
|
Comparison is ready Comparison Summary:
|
On 8/10/17 9:29 AM, Federico De Guio wrote:
Hi,
ok, I don't have objections even if the schedule for the first re-reco
seems tight.
let me just clarify that:
* there is no effect on the current data taking. we are already
running unsuppressed since after TS1
was this done by some other means that are unrelated to this
dropZSmarkedPassed ?
* what comes with this PR is effectively the intended behaviour (no
suppression in HF as in 2016)
This is a bit confusing because this PR apparently changes behavior only
for 2017 pre-TS1 data.
Is that correct?
Please update the description to make it more clear.
* the rechits in HF at the beginning of 2017 were suppressed by
mistake. we are trying to recover the right behaviour for those data
Is post-TS1 in 2017C and later?
…
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#20115 (comment)>, or
mute the thread
<https://github.com/notifications/unsubscribe-auth/AEdcbgIH4GgpHffu6Njd6PF_w1LRw41eks5sWy_sgaJpZM4OziFv>.
|
hi @slava77 |
On 8/10/17 10:13 AM, Federico De Guio wrote:
hi @slava77 <https://github.com/slava77>
yes: at the marking of the raw data was removed at p5 during TS1.
yes: this PR changes only the pre TS1 data.
2017C and later looks fine in prompt already: https://goo.gl/RUi5W8
Thanks.
Now I understand the context of David's earlier comment about era as well,
which apparently implied to have a Run2_2017B era for this setting.
But then if this flag indeed does not change anything for 2017C and later,
then it sounds like the change in this PR does not require an era.
…
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#20115 (comment)>, or
mute the thread
<https://github.com/notifications/unsubscribe-auth/AEdcboHMTPOoWVJ0TYrcdRS494kduZvcks5sWzpTgaJpZM4OziFv>.
|
right. thanks, |
(sorry, wrong button by mistake) |
@deguio : could you please post here the address of one file from 2017C in which I can test this PR on the new data? Thank you |
On 8/13/17 4:12 AM, perrotta wrote:
@deguio <https://github.com/deguio> : could you please post here the
address of one file from 2017C in which I can test this PR on the new
data? Thank you
I think that any file from a good run should work.
JetHT is perhaps better.
/store/data/Run2017C/JetHT/RAW/v1/000/299/649/00000/D08EE651-3C70-E711-884D-02163E011DFC.root
looks OK
run=299649 lumi [168,170], taken July 24.
…
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#20115 (comment)>, or
mute the thread
<https://github.com/notifications/unsubscribe-auth/AEdcbp8wI1W4jce_K-1tJ1ul6A1f0hNlks5sXtoBgaJpZM4OziFv>.
|
yeah, I confirm that. thanks @slava77 |
This PR, when run on 2017B data (200 events from wf 136.783 RunJetHT2017B) gives relevant differences. This is what expected when recovery the non suppressed behavior in HF, I imagine: |
When run on the same number of event (200) in data from RunJetHT2017C (run 299368, lumies 164-165) no difference is spot in the reco output, as anticipated by the proponents of this PR |
+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 (and backports should be raised in the release meeting by the corresponding L2) |
thanks @perrotta |
merge |
earlier this year we realized that there was an effective ZS applied at reco level for HF which was coming from the MarkAndPass flag being set in the RAW data. that flag is now unset and the HF is read and reconstructed in full readout for real starting from TS1. However, in order to recover the wanted non-suppressed behaviour in HF for the early 2017 data in the upcoming re-recos, we would like to merge this change.