-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Dynamic strip reco v3 #10605
Dynamic strip reco v3 #10605
Conversation
- added Loose, Medium and Tight tau ID discriminators with pileup weighted isolation - removal of deprecated tau ID discriminators from pat::Taus
- bug-fix: disable deltaBeta corrections for pileup weighted isolation discriminators
pf charged hadrons and tracks -> solves rare problem with 3-prongs having charge +/-3.
New charge cleaner module for taus
…to DynamicStripReco_v2
…riminators with deltaBeta correction
- updated thresholds for isolation WPs: Loose = 2.5 GeV Medium = 1.5 GeV Tight = 0.8 GeV (no change)
- added Loose, Medium and Tight tau ID discriminators with pileup weighted isolation - removal of deprecated tau ID discriminators from pat::Taus
- bug-fix: disable deltaBeta corrections for pileup weighted isolation discriminators
pf charged hadrons and tracks -> solves rare problem with 3-prongs having charge +/-3.
…riminators with deltaBeta correction
- updated thresholds for isolation WPs: Loose = 2.5 GeV Medium = 1.5 GeV Tight = 0.8 GeV (no change)
double maxRelPhotonSumPt_outsideSignalCone_; | ||
|
||
bool applyFootprintCorrection_; | ||
struct footprintCorrectionType |
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.
types should start with a capital letter (and instances of this type with a lower case) .. there's no need to add "Type" to the name this way.
-1 DAS Error 4.53 step1 DAS Error 140.53 step1 DAS Error 1001.0 step1 DAS Error 1003.0 step1 DAS Error you can see the results of the tests here: |
double minStripEt_; | ||
|
||
std::vector<int> inputPdgIds_; // type of candidates to clusterize | ||
TFormula* etaAssociationDistance_; // size of strip clustering window in eta direction |
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.
use std::unique_ptr
|
||
namespace | ||
{ | ||
TFormula* makeFunction(const std::string& functionName, const edm::ParameterSet& pset) |
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.
it looks like in your code you call only Eval, which is a const function.
change the return type to std::unique_ptr< const TFormula>
@andrewj314 |
@slava77 Thank you so much for taking the time to look over this! I have implemented most of your suggestions, there were just a few I did not understand (mostly those involving using a std::unique_ptr, which I don't know how to do) |
@slava77 Hi Slava, With the exception of 5.1, none of these segfaults include a stacktrace, so I'm unsure to proceed. Should I push the changes to github so you and the other experts can take a look? The only other thing I can think to do is try to pinpoint the source of the segfaults using cout statements, but I imagine there must be a more efficient way to debug this. I've pasted the 5.1 stacktrace below, in case that helps: Thread 1 (process 20314):
|
@andrewj314 launch job with gdb, let it fail and then you should be able to print stack trace. |
Hi AJ, please push to github. |
Thanks so much for being so patient with me on this - here's a link to the branch where I tried to implement Slava's suggestions and got the segfaults: In the meantime, I'm trying to diagnose with GDB |
I think this https://github.com/cms-sw/cmssw/pull/10802/files#r37139859 should fix the crash .. at least it appears to be in about the right place |
-1 |
Here are some plots from #10605 398866a, comparing with CMSSW_7_6_0_pre3 (red is new/with this PR):
In DQM, many distributions for a large number of discriminants have changed. From a technical perspective, the timing and memory per job are not significantly affected.
In AOD, the following products were added/removed [columns are size before, size after, increase, status, and effect in % on total AOD output. These and other tau-related product changes lead to an increase of under 0.5% in the AOD size.
|
Implementation of dynamic strip reco algorithm designed to recover low-pT photons from pizero decays, addition of cleaner module rejecting tau candidates without unit charge, removal of obsolete tau ID discriminators. For an overview of the changes in this PR, see here:
https://indico.cern.ch/event/434527/?filterActive=1&showDate=all&showSession=4#preview:1627952