E/g HLT Pixel Match Variable Producer #13846
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
A new Pull Request was created by @Sam-Harper for CMSSW_8_1_X. It involves the following packages: RecoEgamma/EgammaHLTProducers @Martin-Grunewald, @perrotta, @cmsbuild, @fwyzard, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are list here #13028 |
|
Yes, we'd need a 80X PR as well. |
|
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_1_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @Degano, @smuzaffar |
|
For some reason I suddenly get compilation problems with your new Egamma plugin - since CMSSW_8_1_X_2016-03-28-2300: maybe due to this PR #13820 not playing nice with your new code? |
|
Thanks for the heads up Martin, looking now and will see if I can fix the problem. |
| edm::ParameterSetDescription binParamDesc; | ||
|
|
||
| std::auto_ptr<edm::ParameterDescriptionCases<std::string>> binDescCases; | ||
| binDescCases = |
There was a problem hiding this comment.
To make it compile, remove line 83 and begin line 84 with
auto binDescCases = ...
|
Thanks Chris! I also had to std::move it into the ifValue func but all compiles and is fine now. Should I update this pull request by pushing the new commit to it or make a new one? |
|
The PR is already integrated, so you need to make a new PR. |
|
Concerning TF1 and overhead: perhaps the CMSSW FormulaEvalator could be used? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Dear All,
This is a new producer which makes new variables related to pixel matching for the E/gamma HLT and is a necessary module for the 2016 Z' trigger strategy.
The core of it relies on parameterising the spread of the dPhi1, dPhi2 and dRZ2 (phi and R/Z matching of the supercluster trajectory to the 1st and 2nd pixel hits) for a given electron. This is done via an arbitrary function in arbitrary bins. While we know what function we want to apply, it has been designed to easily change this function in the future while maintaining the exact same python interface (ie old configs still work the same, no confdb reparse needed). Hence the rather abstract interface. Given this is a stream module, my understanding from replies on the fwcore hypernews that a TF1 / TF2 / TF3 is acceptable here. If its not, we can hard code the functions easily enough.
Martin, does this need to be backported to 80X as well for the HLT?
Cheers,
Sam