-
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
Revert decrease of precision introduced in ea3f693d #43253
Conversation
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43253/37632
|
A new Pull Request was created by @maxgalli (Massimiliano Galli) for master. It involves the following packages:
@cmsbuild, @simonepigazzini, @vlimant can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
enable nano |
please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1e7569/35766/summary.html Comparison SummarySummary:
NANO Comparison SummarySummary:
Nano size comparison Summary:
|
+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. @rappoccio, @antoniovilela, @sextonkennedy (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
Wouldn't another solution here be to bin the relevant variables in powers of two? e.g. if |
Hi @nsmith-, nice idea, although I am not completely following. To what extent is this a "solution"? People would like to work with the variables as inputs to MVA algorithms, which usually benefit from smooth input distributions (continuous, differentiable). We had cases of almost catastrophic failure as especially density estimation methods seem to have problems with such "pathological" cases. Rebinning might make bumps go away but it does not help for training a model, right? Or did I misunderstand your suggestion? |
in either case, variables have a finite precision, so nothing is continuous if you bin finely enough.... (eg, use more bins, you will see the same effect in the "correct" plots) |
Hi Nick, thanks. Adding to what @JaLuka98 already correctly said: the binning used in the plots is just a way to show the difference in behaviour between Run2 (precision 10, where we did not see any problem with the training) and Run3 (where the precision was decreased to 8). We already saw that if we choose a finer binning the bumps "go away" (as one would expect), but that does not solve the underlying problem unfortunately. |
The decrease of precision for shower shape variables, introduced in this PR, seems to introduce unwanted features in the shapes.
A comparison of Run2 samples with precision 8 and 10 can be found at the following links: