search

dkriegner / xrayutilities

xrayutilities - a package with useful scripts for X-ray diffraction physicists
http://xrayutilities.sourceforge.io
GNU General Public License v2.0
81 stars 29 forks source link

Q2AngFit: remove workaround for equal bounds once fixed in scipy #101

Closed dkriegner closed 2 years ago

dkriegner commented 4 years ago

1448234 introduces a workaround which should be reverted once scipy/scipy#12433 is fixed

picca commented 4 years ago

Hello,

I added your workaround to the debian package and ran the tests

https://salsa.debian.org/science-team/python-xrayutilities/-/jobs/914603/raw

here an extraction of the log.

test_xrayutilities_q2ang_general (tests.test_examples.TestExampleScripts) ... Traceback (most recent call last):
  File "xrayutilities_q2ang_general.py", line 78, in <module>
    ang, qerror, errcode = xu.Q2AngFit(qvec, hxrd, bounds, startvalues=ang)
  File "/builds/science-team/python-xrayutilities/debian/output/xrayutilities-1.6.0+pypi/.pybuild/cpython3_3.8_xrayutilities/build/xrayutilities/q2ang_fit.py", line 208, in Q2AngFit
    res = scipy.optimize.minimize(_errornorm_q2ang, start,
  File "/usr/lib/python3/dist-packages/scipy/optimize/_minimize.py", line 625, in minimize
    return _minimize_slsqp(fun, x0, args, jac, bounds,
  File "/usr/lib/python3/dist-packages/scipy/optimize/slsqp.py", line 426, in _minimize_slsqp
    g = append(sf.grad(x), 0.0)
  File "/usr/lib/python3/dist-packages/scipy/optimize/_differentiable_functions.py", line 188, in grad
    self._update_grad()
  File "/usr/lib/python3/dist-packages/scipy/optimize/_differentiable_functions.py", line 171, in _update_grad
    self._update_grad_impl()
  File "/usr/lib/python3/dist-packages/scipy/optimize/_differentiable_functions.py", line 91, in update_grad
    self.g = approx_derivative(fun_wrapped, self.x, f0=self.f,
  File "/usr/lib/python3/dist-packages/scipy/optimize/_numdiff.py", line 391, in approx_derivative
    raise ValueError("`x0` violates bound constraints.")
ValueError: `x0` violates bound constraints.
ERROR

So it seems that the workaround is not enought, or I did something wrong ;)

picca commented 4 years ago

Just to be sure for the workaround :)

dpkg-source: info: applying 0005-workaround-bug-in-scipy-1.5.0.patch
dkriegner commented 4 years ago

I saw this error in the unittests as well... will attempt to fix today. Somehow scipy did all this in <1.5.0 internally.

dkriegner commented 4 years ago

This seems to be another bug in scipy (scipy/scipy#11403) and therefore likely has to be ultimately fixed there!

I currently settled for allowing a numerically much higher variation of fixed parameters. This seems to circumvent the problem bug is an very ugly fix which should be removed asap!

Note that likely there are still configurations of input values which would trigger the mentioned bug(s). In the unittests this seems to be not the case anymore!

drew-parsons commented 3 years ago

At https://github.com/scipy/scipy/issues/11403#issuecomment-721464625 they report that the SLSQP part of the problem is fixed by https://github.com/scipy/scipy/pull/13009 . That's in scipy master but not in the 1.5.4 release.

dkriegner commented 3 years ago

thanks for your message. in fact I have seen this comment and tested my issue with scipy-master. The problem is still not solved!

I have, however, after the commit in https://github.com/scipy/scipy/pull/13009 adjusted my code and fixed the parameters into a window of +/- xu.config.EPSILON by bounds and in addition totally fixed them by additional equality constraints.

For scipy <1.5 all this was not necessary and equal lower and upper bound could be used. I think the remaining issue is described in https://github.com/scipy/scipy/issues/12433, but xrayutilities-master should work with scipy-master as intented.

I will keep this open and retest with recent scipy-master from time to time

Khodunov commented 3 years ago

I have the same issue with SLSQP (scipy 1.5.4), but couldn't replicate it with a simpler function. Somehow when I change the minimized function, ValueError disappears (it must point to the bug, right?)

dkriegner commented 3 years ago

@Khodunov I am not sure what you are proposing? As far as I understand SLSQP is needed for this application to support equality and inequality constraints.

With a simpler function you mean a different minimize method? In this case I am afraid this is not an option here

Khodunov commented 3 years ago

@dkriegner

With a simpler function you mean a different minimize method? In this case I am afraid this is not an option here

No, I mean different function which is minimized via SLSQP. I have a sophisticated function, which I minimize via SLSQP, and it gives me the error. Then I change the function to the simple one (like |x|^2) and the error vanishes. This way I know that the error is in minimize, not in initial value (because I didn’t change initial values).

By the way, I downgraded scipy to 1.4.1 and lived happily ever after

Khodunov commented 3 years ago

Oops, it made the quote bigger than intended

dkriegner commented 3 years ago

I see what you mean. You should report this to the scipy developers if you did not do already. Obviously the problem I intend so solve can't be simplified. Downgrading works for me as well, but this will not be a sustainable solution

dkriegner commented 3 years ago

Oops, it made the quote bigger than intended

I fixed it

Khodunov commented 3 years ago

I see what you mean. You should report this to the scipy developers if you did not do already.

Well, I thought I did via posting here :))

dkriegner commented 2 years ago

This should be fixed now in the main branch and be effective once scipy>1.8.0 is available.