BUG: Fixed broken dogbox trust-region radius update

[nmayorov]

I was solving some least-squares problems and bumped into quite a bug, which blocks dogbox solver progress.

Explanation: if the trial step was unsuccessful then trust-region radius is updated as Delta = 0.25 * step_h_norm, but in dogbox trust regions are rectangular, thus step_h_norm must be computed with infinity norm, otherwise Delta may not shrink and we get into infinite loop.

Please merge this fix asap.

[ev-br]

How hard would it be to add a test for this?

[nmayorov]

Not very difficult, I can include one rather small problem. I guess you recommend me to do it)

btw: I hope to send a narrative documentation update for least squares in a few days.

[nmayorov]

Evgeni, a test is here.

I noticed one thing: when using finite diff. Jacobian estimation, we have to set max_nfev (if we want to) for method='lm' much higher than for other methods (as MINPACK counts function calls for Jacobian estimation). It is not convenient. Maybe introduce automatic adjustment for that or it would be only worse?

[ev-br]

The fix looks right, but the test doesn't seem to test it: if I cherry-pick nmayorov@71d6c5b on top of master, optimize tests still pass.

[nmayorov]

@ev-br hmm, I deliberately tested for this before sending. Sorry, I will recheck.

[nmayorov]

Are you sure? When I remove ord=np.inf I immediately get:

FAIL: test_least_squares.TestDogbox.test_bvp
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/nmayorov/anaconda/lib/python3.4/site-packages/nose/case.py", line 198, in runTest
    self.test(*self.arg)
  File "/Users/nmayorov/PythonDev/scipy/scipy/optimize/_lsq/tests/test_least_squares.py", line 353, in test_bvp
    assert_(res.nfev < max_nfev)
  File "/Users/nmayorov/anaconda/lib/python3.4/site-packages/numpy/testing/utils.py", line 53, in assert_
    raise AssertionError(smsg)
AssertionError

[ev-br]

Rrright.

OK, when I run the tests with the file name, it fails indeed.
Which prompts the question: Is it actually tested?
Let's see:

• I checked out your branch, then $ python runtests.py -s optimize: Ran 387 tests in 7.992s.
• Now, $ rm scipy/optimize/_lsq/tests/* , and rerun tests: Ran 387 tests in 7.935s.

Can you please check this Nikolay?

[pv]

There's no `config.add_data_dir('tests')` in _lsq/setup.py, so the test files don't end up installed.

[pv]

The numpy test runner also doesn't seem to run the tests on scipy.optimize.test() for some reason, even if installed. (Maybe because the submodule name starts with _ --- would be weird though?)

[nmayorov]

I always used nosetests .... I will modify setup.py as Pauli suggested.

[nmayorov]

@pv I'm struggling to get nose to test _lsq when running nosetests scipy.optimize. Can you help with that?

Maybe just put all tests in optimize/tests?

[ev-br]

I don't understand distutils (or setuptools, whichever is this), but this change seems to do it:

$ git diff
diff --git a/scipy/optimize/_lsq/setup.py b/scipy/optimize/_lsq/setup.py
index b9222a0..49453ac 100644
--- a/scipy/optimize/_lsq/setup.py
+++ b/scipy/optimize/_lsq/setup.py
@@ -6,6 +6,7 @@ def configuration(parent_package='', top_path=None):
     config = Configuration('_lsq', parent_package, top_path)
     config.add_extension('givens_elimination',
                          sources=['givens_elimination.c'])
+    config.add_data_dir('tests')
     return config

[ev-br]

With this, I get the number of tests run in scipy.optimize up to five hundred, the test time jumps threefold (from ~7 sec to ~20sec), and there's a failure on numpy 1.6.2:

======================================================================
ERROR: test_common.test_reflective_transformation
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/br/virtualenvs/scipy-dev/local/lib/python2.7/site-packages/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/home/br/installs/scipy/build/testenv/lib/python2.7/site-packages/scipy/optimize/lsq/tests/test_common.py", line 230, in test_reflective_transformation
    x, g = reflective_transformation(y, lb, np.array([np.inf, np.inf]))
  File "/home/br/installs/scipy/build/testenv/lib/python2.7/site-packages/scipy/optimize/_lsq/common.py", line 526, in reflective_transformation
    x[mask] = np.minimum(y[mask], 2 * ub[mask] - y[mask])
TypeError: array cannot be safely cast to required type

----------------------------------------------------------------------
Ran 506 tests in 21.462s

FAILED (KNOWNFAIL=1, errors=1)

[nmayorov]

So basically you added the line config.add_data_dir('tests') and then python runtests.py -s optimize worked? Unfortunately in my setup it doesn't change anything. Not sure what I should do.

[ev-br]

Nope, that was a stale build :-(.
What worked was: (i) add the line above, (ii) git mv scipy/optimize/_lsq scipy/optimize/lsq and rename the extension in setup.py of optimize and optimize/lsq. So it's a leading underscore which somehow throws it off.

[pv]

Probably amount to a bug in numpy.testing, there might be some regexp discarding underscores.
Never looked into that code, though, so I don't know more.

I'd just move the tests to scipy/optimize/tests for now. Submodule is nicer with underscore...

[nmayorov]

Thanks, Pauli. Evgeni are you OK with the decision to move tests?

[pv]

Skipping _ prefix is a nose bug: https://github.com/nose-devs/nose/blob/master/nose/loader.py#L169

[ev-br]

Evgeni are you OK with the decision to move tests?

Sounds like the best way out indeed.

[ev-br]

W.r.t. the question of nfev, let's separate it: #5566

[nmayorov]

@ev-br

I modified tests of reflective transformation as not to crash on numpy 1.6.2, it doesn't affect real performance as optimization code works with float arrays (the issue was dtype mismatch which isn't handled by numpy 1.6.2)

So I think it's ready to be merged. The error on TravisCI is not related to the changes in this PR.

[ev-br]

Indeed, CI errors seem unrelated and should be fixed by gh-5572.
But in Wine, I get two more test failures

======================================================================
FAIL: test_least_squares.TestDogbox.test_grad
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Python27\lib\site-packages\nose-1.1.2-py2.7.egg\nose\case.py", line 197, in runTest
    self.test(*self.arg)
  File "C:\Python27\lib\site-packages\scipy\optimize\tests\test_least_squares.py", line 567, in test_grad
    assert_equal(res.grad, 2 * x)
  File "C:\Python27\lib\site-packages\numpy\testing\utils.py", line 256, in assert_equal
    return assert_array_equal(actual, desired, err_msg, verbose)
  File "C:\Python27\lib\site-packages\numpy\testing\utils.py", line 707, in assert_array_equal
    verbose=verbose, header='Arrays are not equal')
  File "C:\Python27\lib\site-packages\numpy\testing\utils.py", line 636, in assert_array_compare
    raise AssertionError(msg)
AssertionError: 
Arrays are not equal

(mismatch 100.0%)
 x: array([ 4.])
 y: array(4.0)

======================================================================
FAIL: test_least_squares.TestTRF.test_grad
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Python27\lib\site-packages\nose-1.1.2-py2.7.egg\nose\case.py", line 197, in runTest
    self.test(*self.arg)
  File "C:\Python27\lib\site-packages\scipy\optimize\tests\test_least_squares.py", line 567, in test_grad
    assert_equal(res.grad, 2 * x)
  File "C:\Python27\lib\site-packages\numpy\testing\utils.py", line 256, in assert_equal
    return assert_array_equal(actual, desired, err_msg, verbose)
  File "C:\Python27\lib\site-packages\numpy\testing\utils.py", line 707, in assert_array_equal
    verbose=verbose, header='Arrays are not equal')
  File "C:\Python27\lib\site-packages\numpy\testing\utils.py", line 636, in assert_array_compare
    raise AssertionError(msg)
AssertionError: 
Arrays are not equal

(mismatch 100.0%)
 x: array([ 4.])
 y: array(4.0)

Looks like these would be easy to fix.

[nmayorov]

@ev-br I changed those tests.

[ev-br]

Great, thanks Nikolay!