-
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
Set the SOI for HCAL TP samples when packing. #19804
Conversation
A new Pull Request was created by @matz-e (Matthias Wolf) for master. It involves the following packages: EventFilter/HcalRawToDigi @perrotta, @cmsbuild, @slava77, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
auto raw = qiedf->sample(iTS).raw(); | ||
// Add SOI information | ||
if (iTS == qiedf->presamples()) | ||
raw |= 0x4000; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
better to define a mask with this value as a const in the TPHeaderSpec
namespace
@cmsbuild please test |
The tests are being triggered in jenkins. |
+1 The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
Pull request #19804 was updated. @perrotta, @cmsbuild, @slava77, @davidlange6 can you please check and sign again. |
@cmsbuild please test |
The tests are being triggered in jenkins. |
+1 The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
Looking at jenkins comparisons, there are some changes in 2017 MC related to the TP DQM Some are somewhat clearly expected, e.g. as but then many more are not obviously going in the right direction, e.g. @matz-e please clarify what is happening and if the diff is expected. |
@slava77 The SOI bit indicates the sample that generates the L1A. If this bit is not set properly, the default value for the number of presamples (0) is used, and the values of the TP displayed in these DQM plots are those of early OOT pileup. In an MC dataset with no pileup simulation, these TP energies will be negligible. For example, in this plot the maximum "Raw ET" before this PR corresponds to a TP energy of around 1 GeV: Also, the HF plot you reference is an input to the HF minimum bias trigger: before this PR, TTbar TPs are insufficiently energetic to fire the minimum bias trigger. The L1T DQM plots are simply not filled correctly in general. This is most easily seen in the corresponding 2D plots, where the energy of the TP received by Layer-1 is always zero: In data, these plots compare the TPs sent by the HCAL uHTRs with the same TPs recorded by Layer-1, in order to monitor link errors. I'm not sure if the MC bothers to make an exact duplicate of the HCAL TP in the SOI or simply uses the HCAL TPs directly (in which case the "Layer 1 received" TPs may not even exist). Perhaps @nsmith- can comment? |
Hi @christopheralanwest,
So these histograms typically get filled with just the negative of the TP Et spectrum, which isn't necessarily useless I guess. To answer your specific question, though, all plots are filled with |
On 7/24/17 1:35 AM, Nicholas Smith wrote:
So these histograms typically get filled with just the negative of the
TP Et spectrum, which isn't necessarily useless I guess.
good to know. This explains the changes in the "difference" plots
|
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 requires discussion in the ORP meeting before it's merged. @davidlange6, @smuzaffar |
+1 |
See title. The information is accessed on a per-sample basis when unpacking, but the raw samples in MC don't have the corresponding bit set. Set it when packing.