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
Alignment filters 80X #12776
Alignment filters 80X #12776
Conversation
Implemented as EDFilter that checks the SiStripLatencyRcd.
A new Pull Request was created by @ghellwig (Gregor Mittag) for CMSSW_8_0_X. It involves the following packages: Alignment/CommonAlignment @diguida, @cerminar, @cmsbuild, @franzoni, @mmusich, @davidlange6 can you please review it and eventually sign? Thanks. Following commands in first line of a comment are recognized
|
please test |
The tests are being triggered in jenkins. |
float | ||
MagneticFieldFilter::currentToField(const float& current) const { | ||
return 2.084287e-04 * current + 1.704418e-02; | ||
} |
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.
@ghellwig magic numbers, please define them as const and clarify how they are extracted
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.
You mean, I need to put the coefficients as constant data members with a comment where they come from?
It is in principle written in the documentation at the top of the code.
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
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.
ok, I'll add these numbers as constants and will also update the backports of this PR.
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.
thanks!
Includes an (approximate) EDFilter and a script to create JSON files according to the content of the run registry.
1e3a5c1
to
5524b9a
Compare
Hi Marco, |
@ghellwig thanks! |
please test |
The tests are being triggered in jenkins. |
+1 |
This pull request is fully signed and it will be integrated in one of the next CMSSW_8_0_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @Degano, @smuzaffar |
+1 |
Alignment filters 75X (Backport of #12776)
Alignment filters 76X (Backport of #12776)
Alignment filters 74X (Backport of #12776)
Tools to streamline the alignment work-flow and the corresponding validation.
These tools have been presented in
https://indico.cern.ch/event/442133/session/0/contribution/4/attachments/1203128/1751921/talk.pdf