-
-
Notifications
You must be signed in to change notification settings - Fork 25.9k
FIX GaussianMixture
now normalizes weights_
directly instead of by n_samples
#23034
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
Conversation
GaussianMixture
now normalizes weights_
directly instead of by n_samples
GaussianMixture
now normalizes weights_
directly instead of by n_samples
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 for the PR @Micky774
Note for future reviewers: |
Co-authored-by: Jérémie du Boisberranger <34657725+jeremiedbb@users.noreply.github.com>
…into gmix_norm
Co-authored-by: Jérémie du Boisberranger <34657725+jeremiedbb@users.noreply.github.com>
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.
LGTM
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.
Thank you for the PR!
LGTM
GaussianMixture
now normalizes weights_
directly instead of by n_samples
GaussianMixture
now normalizes weights_
directly instead of by n_samples
…y `n_samples` (scikit-learn#23034) Co-authored-by: Jérémie du Boisberranger <34657725+jeremiedbb@users.noreply.github.com>
Reference Issues/PRs
Fixes #23032
What does this implement/fix? Explain your changes.
The implementation on
main
normalizes theweights_
attribute (which should form a pmf) byn_samples
which lead to floating-point precision errors when fitting a single-component gaussian mixture on few samples (e.g.n_samples < 32
). Now theweights_
array is normalized directly w.r.t its sum reduction. Added non-regression test.Any other comments?