[MRG+1] Fixes n_iter_without_progress and min_grad_norm in TSNE #6497
Conversation
|
Seems to be ok for me. Could you add a test for |
|
The build errors are not related. I will see how I can test |
|
Ok, I added a test for |
|
Sorry to butt in, but I'm wondering if this P.R. slipped through the cracks, or if there's another reason it hasn't been merged? I just got hit by #6450 myself, and it's pretty hard to work around. |
|
Yes, slipped through the cracks. Thanks for resurrecting it. |
|
|
||
| # The gradient norm can be smaller than min_grad_norm at most once, | ||
| # because in the moment it becomes smaller the optimization stops | ||
| assert_less_equal(n_smaller_gradient_norms, 1) |
|
I'm not loving the text hacking, but as a fix I think this is good. |
|
@AlexanderFabisch, could you please review the test for |
| sys.stdout = old_stdout | ||
|
|
||
| # The output needs to contain the value of n_iter_without_progress | ||
| assert("did not make any progress during the " |
There was a problem hiding this comment.
You can use nose.tools.assert_in
|
@colinmorris did this PR fix your issue? |
| Maximum number of iterations without progress before we abort the | ||
| optimization. | ||
| optimization. If method='barnes_hut' this parameter is fixed to |
There was a problem hiding this comment.
This behavior is somewhat peculiar and we could work around it, but let's leave it for now.
|
It fixes a bug. So we should merge it. Maybe we should add the newline at the end of the test file. |
# Conflicts: # doc/whats_new.rst
Fixes #6450.
The parameters
n_iter_without_progressandmin_grad_normare now correctly used and a test was added to ensure that n_iter_without_progress is used correctly.