-
Notifications
You must be signed in to change notification settings - Fork 20
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
Update beam energy and thresholds in simpleTrigger.py #1245
Conversation
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.
The failing tests are failing because of the change in trigger thresholds. All of them only have two histograms changing:
Trigger_Trigger_Trigger.pass.pdf
Trigger_Trigger_Trigger.EcalEcut.pdf
I downloaded the artifacts and manually merged the failed plots from the different validation samples into a document to browse. The changes look as expected to my eye.
- The Ecal Cut distributions scale with 8GeV and the electron count rather than 4GeV and the electron count.
- There is an increase in trigger-passing for all distributions following the more appropriate (looser) trigger cut being applied.
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.
I agree with Tom's interpretation of the plots he shared - they all seem reasonable! Looks good.
Thanks @tomeichlersmith for reviewing the CI artifacts and verifying that we see only the expected changes. @jpasc27 will you have a chance to go over the changes you did to your config and make sure they are plugged into the defaults for the relevant processors, as we discussed in #1229 ? I don't want to close this issue before there is a full solution to that issue. |
hey @jpasc27 , just a reminder about my question to you in this issue. we're trying to close all open PRs within about a week, before we pull all the submodules directly into |
Sorry, completely forgot about this (as I'm sure you can tell)! I will check my code to answer your question on Monday, that way this can be resolved before your Thursday deadline. |
awesome, thanks @jpasc27. |
Added in some changes to the trigger processor defaults that I came across when setting up for Ecal PN production. Adding this to the list of changes needed for the filters and biasing etc, tracked in #1229.
with this and if people are happy with c99cdcb I think this PR can be marked as "Ready for review" and doesnt have to stay Draft, right? |
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.
I'm happy with these changes and (as before) the plots that are failing the tests are related to the trigger and how it is being applied. only the validation samples that have changed to 8GeV are failing now which is to be expected.
TeX which made the above PDF
\documentclass{beamer}
\usepackage{graphicx}
\begin{document}
\begin{frame}
\centering
Failing Plots
\end{frame}
\begin{frame}
\centering
Inclusive
\end{frame}
\begin{frame}
\includegraphics[width=\textwidth]{inclusive/Trigger_Trigger_Trigger.EcalEcut.pdf}
\end{frame}
\begin{frame}
\centering
Signal
\end{frame}
\begin{frame}
\includegraphics[width=\textwidth]{signal/Trigger_Trigger_Trigger.EcalEcut.pdf}
\end{frame}
\begin{frame}
\includegraphics[width=\textwidth]{signal/Trigger_Trigger_Trigger.pass.pdf}
\end{frame}
\end{document}
Added in some changes to the trigger processor defaults that I came across when setting up for Ecal PN production. Adding this to the list of changes needed for the filters and biasing etc, tracked in #1229. This will cause changes to gold histograms insofar as there are any for the trigger processor (less strict threshold when properly taking the higher beam energy into account --> more will pass)
What are the issues that this addresses?
Partial solution to #1229.