[WIP] FIX Projected Gradient NMF stopping condition#2557
[WIP] FIX Projected Gradient NMF stopping condition#2557vene wants to merge 1 commit intoscikit-learn:masterfrom
Conversation
There was a problem hiding this comment.
I find the code a bit hard to follow. I would rather define variables proj_grad_W and proj_grad_H and compute the Frobenius norms of those.
|
The non-negative least squares benchmark notebook version of the solver diverged a little bit. Most importantly the regularization is more properly done (but this makes the objective function and the parametrization different). Should I bring it in to this PR? |
|
Could you summarize how it will impact the parametrization? Do you think there is a way to maintain backward compat with a deprecation cycle? |
|
The default result of the estimator would change anyway in many cases because of the change in stopping conditions. Should I just put the refactored solver in this PR to discuss it? Make a separate PR? |
|
I am fine with bringing it into this PR to make it easier to see the changes. |
|
Should this be closed in light of #4852? |
|
yes. Closing |
I caught a bug in the PG NMF stopping condition. All in all it seems that this solver is not at all bad, but I need to understand it properly.