Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

shgo error since scipy 1.8.0.dev0+1529.803e52d #14589

Closed
ericpre opened this issue Aug 15, 2021 · 13 comments · Fixed by #16506
Closed

shgo error since scipy 1.8.0.dev0+1529.803e52d #14589

ericpre opened this issue Aug 15, 2021 · 13 comments · Fixed by #16506
Labels
defect A clear bug or issue that prevents SciPy from being installed or used as expected scipy.optimize
Milestone

Comments

@ericpre
Copy link

ericpre commented Aug 15, 2021

shgo raises an error with the following example since scipy 1.8.0.dev0+1529.803e52d.

This error was noticed in the hyperspy test suite (https://github.com/hyperspy/hyperspy-extensions-list/actions/runs/1111267440) and previous scipy development version (1.8.0.dev0+1503.8647ee9) didn't raise an error. Scipy development version are installed from https://pypi.anaconda.org/scipy-wheels-nightly/simple.

Reproducing code example:

import sys

import numpy as np
import scipy
from scipy.optimize import shgo

print(scipy.__version__, np.__version__, sys.version_info)


def func(x_values, param):
    a, b = param
    return a*x_values + b


def errfunc(param, y):
    return func(x_values, param) - y


def errfunc_sq(param, y, weights=None):
    if weights is None:
        weights = 1.0
    return ((weights * errfunc(param, y)) ** 2).sum()

x_values = np.arange(100)
y_values = func(x_values, (2, 10)) + np.random.random(x_values.size)

results = shgo(errfunc_sq,
               bounds=((-100, 100), (-100, 100)),
               args=(y_values, None)
               )

print(results)

Error message:

1.8.0.dev0+1529.803e52d 1.21.1 sys.version_info(major=3, minor=8, micro=10, releaselevel='final', serial=0)
Traceback (most recent call last):
  File "/opt/miniconda3/envs/test_env/lib/python3.8/site-packages/scipy/optimize/_shgo_lib/triangulation.py", line 630, in __getitem__
    return self.cache[x]
KeyError: (0, 0)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "untitled0.py", line 34, in <module>
    results = shgo(errfunc_sq,
  File "/opt/miniconda3/envs/test_env/lib/python3.8/site-packages/scipy/optimize/_shgo.py", line 420, in shgo
    shc.construct_complex()
  File "/opt/miniconda3/envs/test_env/lib/python3.8/site-packages/scipy/optimize/_shgo.py", line 733, in construct_complex
    self.iterate()
  File "/opt/miniconda3/envs/test_env/lib/python3.8/site-packages/scipy/optimize/_shgo.py", line 876, in iterate
    self.iterate_complex()
  File "/opt/miniconda3/envs/test_env/lib/python3.8/site-packages/scipy/optimize/_shgo.py", line 895, in iterate_hypercube
    self.HC = Complex(self.dim, self.func, self.args,
  File "/opt/miniconda3/envs/test_env/lib/python3.8/site-packages/scipy/optimize/_shgo_lib/triangulation.py", line 25, in __init__
    self.n_cube(dim, symmetry=symmetry)
  File "/opt/miniconda3/envs/test_env/lib/python3.8/site-packages/scipy/optimize/_shgo_lib/triangulation.py", line 76, in n_cube
    self.C0.add_vertex(self.V[origintuple])
  File "/opt/miniconda3/envs/test_env/lib/python3.8/site-packages/scipy/optimize/_shgo_lib/triangulation.py", line 634, in __getitem__
    xval = Vertex(x, bounds=self.bounds,
  File "/opt/miniconda3/envs/test_env/lib/python3.8/site-packages/scipy/optimize/_shgo_lib/triangulation.py", line 557, in __init__
    self.f = func(x_a, *func_args)
  File "/opt/miniconda3/envs/test_env/lib/python3.8/site-packages/scipy/optimize/optimize.py", line 464, in function_wrapper
    fx = function(np.copy(x), *(wrapper_args + args))
TypeError: errfunc_sq() takes from 2 to 3 positional arguments but 5 were given

Scipy/Numpy/Python version information:

1.8.0.dev0+1529.803e52d 1.21.1 sys.version_info(major=3, minor=8, micro=10, releaselevel='final', serial=0)

@tupui
Copy link
Member

tupui commented Aug 15, 2021

Thanks for the report. @Stefan-Endres is working on a PR that is going to refactor quite some things. Stefan, does your PR address this?

@tupui tupui added scipy.optimize defect A clear bug or issue that prevents SciPy from being installed or used as expected labels Aug 15, 2021
@Stefan-Endres
Copy link
Contributor

Thank you for the report @ericpre. @tupui this is a new bug, but on the old caching please keep the issue open I will fix this in the new caching library.

@ericpre
Copy link
Author

ericpre commented Sep 4, 2021

It seems that the regression has been introduced in #14274.

@jjramsey
Copy link
Contributor

jjramsey commented Apr 11, 2022

This bug got me too. FWIW, I reproduced it with the following code:

import scipy.optimize as so

def obj_fun(x, a, b):
    return (x[0]/a)**2 + (x[1]/b)**2

def main():
    a = 1.0
    b = 1.0

    bounds = [(-1.0, 1.0),
              (-1.0, 1.0)]

    result = so.shgo(obj_fun,
                     bounds,
                     args = (a,b))

    if result.success:
        print(f"x = {result.x} and obj_fun(x) = {result.x}")
    else:
        print(f"""
 Message from minimizer: {result.message}
 Status code from minimizer: {result.status}
 Optimization "solution" at failure: {result.x}
        """)

if __name__ == "__main__":
    main()

It looks like the problem is in how so.shgo handles the args parameter. It appears to double the arguments fed to the objective function. This problem does not occur in SciPy 1.5.0.

@jjramsey
Copy link
Contributor

I think I've found a solution. On line 900 of scipy/optimize/_shgo.py, change

self.HC = Complex(self.dim, self.func, self.args,

to

self.HC = Complex(self.dim, self.func, (),

What's happening is that since self.func already contains the contents of self.args, when Complex is constructed, it effectively includes self.args twice, so when it calls the objective function, it effectively calls it as func(x, *(args+args)).

Unfortunately, an alternative like setting self.func to _wrap_scalar_function(func, ()) doesn't work, since there are other places in the code where it's assumed that self.func wraps the contents of self.args. self.args is also used in other places in the code, so setting it to () probably won't work either. If the sampling method is changed from the default simplicial, SHGO still appears to work properly, regardless of my change. The change I recommend appears to be the most minimal change needed to get SHGO to work with externally supplied arguments again.

@tupui
Copy link
Member

tupui commented Apr 12, 2022

There are large development in progress changing the whole architecture of shgo. We will keep these in mind.

@jjramsey
Copy link
Contributor

Understood, but bear in mind that this bug is not only almost a year old, but breaks some very basic documented functionality. A minimal fix now will provide an indicator to those making large changes of what should or shouldn't be done with function arguments, while solving a problem facing end users of SHGO now.

@tupui
Copy link
Member

tupui commented Apr 12, 2022

@jjramsey you are welcomed to make such a fix.

@tupui
Copy link
Member

tupui commented Apr 12, 2022

The large developments are happening here #14013

@jjramsey
Copy link
Contributor

Looks like the pull request for those large developments has been stalled since Jul 26, 2021, apparently due to some automatic tests failing for some unclear reason. Any way to rerun the tests to see if the failure was a fluke (or perhaps some fault in the Azure pipelines)?

@tupui
Copy link
Member

tupui commented Apr 13, 2022

@jjramsey the PR is not staled due to failing tests but it's conditioned to maintainers time to work on SciPy. Most of us are volunteers and don't have much time to work on SciPy.

@abianco88
Copy link

Hi all! I was experimenting with shgo and I suspect I am facing the same issue originally reported by @ericpre about a year ago. I hadn't found this open issue in the tracker before, so I posted a question on StackOverflow (here is the link). (Please note that my posted code clearly has a mistake in the objective function because it lacks the "()" at the end of the sum method, as a responder rightly reported.)
Is there any quick fix to make the SciPy SHGO implementation usable when one needs to pass additional arguments to the objective function? I understand that there are big developments in progress for shgo, but even when the PR is completed, it won't be available until the next version of SciPy is released, will it?

@WarrenWeckesser
Copy link
Member

WarrenWeckesser commented Jun 30, 2022

@abianco88, I posted an answer to your question on Stackoverflow, the essential part of which is:

You can avoid the bug in shgo by avoiding the use of the args parameter. Instead of using args, use a wrapper of fobj that captures y and z in a closure. A simple way to do that is with a lambda expression: change the first argument of shgo from func=fobj to func=lambda x, y=y, z=z: fobj(x, y, z), and remove the args parameter from the call.

o-alexandre-felipe added a commit to o-alexandre-felipe/scipy that referenced this issue Jun 30, 2022
Solution suggested by jjramsey in
scipy#14589 (comment)

The answer at the time was that an important refactor was in progress
scipy#14013

This pull request is now closed and the bug persists in version 1.8.1 accordingly to this stack overflow question
https://stackoverflow.com/q/72794609/12750353
mdhaber pushed a commit that referenced this issue Oct 9, 2022
* Fix bug #14589 where arguments to objective function aren't passed properly

Co-authored-by: J. J. Ramsey <jjramsey@pobox.com>
@mdhaber mdhaber added this to the 1.9.3 milestone Oct 9, 2022
ev-br pushed a commit to ev-br/scipy that referenced this issue Oct 14, 2022
* Fix bug scipy#14589 where arguments to objective function aren't passed properly

Co-authored-by: J. J. Ramsey <jjramsey@pobox.com>
tylerjereddy pushed a commit to tylerjereddy/scipy that referenced this issue Oct 16, 2022
* Fix bug scipy#14589 where arguments to objective function aren't passed properly

Co-authored-by: J. J. Ramsey <jjramsey@pobox.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
defect A clear bug or issue that prevents SciPy from being installed or used as expected scipy.optimize
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants