[WIP] FIX Projected Gradient NMF stopping condition #2557
Conversation
| proj_norm = norm(np.r_[gradW[np.logical_or(gradW < 0, W > 0)], | ||
| gradH[np.logical_or(gradH < 0, H > 0)]]) | ||
| # stopping condition on the norm of the projected gradient | ||
| proj_norm = ( |
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.