-
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
L1Trigger/TrackerDTC Debugged and Analyzer added. #29520
Conversation
The code-checks are being triggered in jenkins. |
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-29520/14782
Code check has found code style and quality issues which could be resolved by applying following patch(s)
|
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-29520/14783
|
A new Pull Request was created by @tschuh for master. It involves the following packages: L1Trigger/TrackerDTC @cmsbuild, @rekovic, @benkrikler, @kpedro88 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
please test |
The tests are being triggered in jenkins. |
* \author Thomas Schuh | ||
* \date 2020, Apr | ||
*/ | ||
class Analyzer : public one::EDAnalyzer<one::WatchRuns, one::SharedResources> { |
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.
this seems like it should migrate toward the validation subsystem eventually (as always, sooner is better than later)
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 will get in contact with the person making DQM plots in our group to asses if, what and how. Will come back when we have a plan but I cannot promise that we will do that quick enough for 11.1
Is it ok if that lives for the short term here?
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.
Yes, we've allowed it before (on a temporary basis)
+1 |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
Pull request #29520 was updated. @cmsbuild, @rekovic, @benkrikler, @kpedro88 can you please check and sign again. |
please test |
The tests are being triggered in jenkins. |
+1 |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
+upgrade |
+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. @silviodonato, @dpiparo (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
PR description:
I added an EDAnalyzer to study the DTC performance and fixed some bugs I have introduced during initial PR review. The analyzer is not part of the standard sequences.