-
Notifications
You must be signed in to change notification settings - Fork 53
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
KDE Boundary Mirror Logic #371
KDE Boundary Mirror Logic #371
Conversation
…e boundaries, instead of the minimum/maximum of the data; adapt docstring for allowed bandwidth methods for ExactKDE and GridKDE
for more information, see https://pre-commit.ci
Hey looks good! I may have missed something in the logic though, but that can be on my side |
Co-authored-by: Jonas Eschle <jonas.eschle@cern.ch>
Co-authored-by: Jonas Eschle <jonas.eschle@cern.ch>
That seems fine so far. If you think the implementation is finished, just remove the WIP in the name and request a review. Make sure to tick of the boxes, so you see that everything is done (e.g. add an entry in the CHANGELOG) |
Hey, was just wondering what's the status of this? |
Hi im sorry for the stall. im on it today again. |
In the changelog i have added this:
to the bugfixes/small changes section. I cannot |
Some coverage tests are showing up as failed. |
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 a lot, this looks good! LGTM
Fixes #
Proposed Changes
KDE1DimExact
,KDE1DimGrid
,KDE1DimFFT
, andKDE1DimISJ
to use the boundary of the observable space instead of the minimum/maximum of the data. This prevents unwanted ''bumps'' in the pdf when the data falls to zero inside of the observable range and both edges are mirrored.KDE1DimExact
andKDE1DimGrid
classes to correctly reflect on the available bandwidth options.Tests added
test_kde_funcs.py
Checklist