tBuLi / symfit

Symbolic Fitting; fitting as it should be.
http://symfit.readthedocs.org
MIT License
235 stars 18 forks source link

README example not working (symfit 0.4.6) #232

Closed ptrebert closed 5 years ago

ptrebert commented 5 years ago

Hi, I discovered the symfit package today and tried testing it in a Conda environment with symfit 0.4.6 and sympy 1.4 (numpy 1.13.1). I pasted the example from the readme

from symfit import parameters, Fit, Equality, GreaterThan

x, y = parameters('x, y')
model = 2 * x * y + 2 * x - x**2 - 2 * y**2
constraints = [
    Equality(x**3, y),
    GreaterThan(y, 1),
]
fit = Fit(- model, constraints=constraints)
fit_result = fit.execute()

in a Jupyter notebook and got the following:

   213         try:
   214             return getattr(obj, self.cache_attr)
    215         except AttributeError:
AttributeError: 'Constraint' object has no attribute '_cached_numerical_components'
During handling of the above exception, another exception occurred:
[...omitted...]
    343     def _print_Variable(self, expr):
    344         return self._print(expr.symbol)
    345 
    346     def _print_Statement(self, expr):

AttributeError: 'Variable' object has no attribute 'symbol'

Looks a bit like #186 , but as that is marked as closed, I opened a new issue.

Best, Peter

Jhsmit commented 5 years ago

symfit 0.4.6 requires sympy <= 1.1.1. Can you try with this sympy version?

ptrebert commented 5 years ago

This does not seem to be the case - if I try switching to sympy=1.1.1, the conda env cannot be created:

UnsatisfiableError: The following specifications were found to be in conflict:
  - symfit=0.4.6 -> sympy[version='>=1.2']
  - sympy=1.1.1
Jhsmit commented 5 years ago

It looks like the conda recipe used the requirements.txt from the master branch at that time and not from the 0.4.6 release. The 0.4.6 requirements.txt is this file: https://github.com/tBuLi/symfit/blob/41425fd88bdd5ed7d2b661689e2189e1de421180/requirements.txt

If you do pip install sympy==1.1.1 in your current conda env to change to that version it should work. ping @tBuLi , conda-feedstock recipy maintainer.

ptrebert commented 5 years ago

I see; indeed, with a forced setup of sympy v1.1.1 the example code works. Thanks for your help.

Just to be complete, the example code gives a RuntimeWarning: invalid value encountered in double_scalars return 1 - SS_res/SS_tot; this was already sort of pointed out in #165, but I can't figure out if that is expected or not based on the comments in the other issue. Also, as result, I get OrderedDict([('x', 1.0), ('y', 1.0)]), which seems to violate the constraint GreaterThan(y, 1) if I read that correctly (y > 1).

tBuLi commented 5 years ago

Hi @ptrebert, thank you for bringing this issue to our attention! I'll make some changes to conda to prevent this in the future.

In answer to your other questions, the RuntimeWarning can be safely ignored, and in fact it will be fixed once PR #221 is merged. It is raised because currently we always try to compute the R^2 of the fit, even if it doesn't make sense to try.

About the example, I do not now remember what the solution to that problem was, but of course strict inequality is a tricky thing numerically. Try setting the intial guesses for the fit to something different, just to be sure. For example, set

x.value = -1
y.value = 1.2

to be sure something is happening. If you then still get the same answer, I'd write it down to numerical errors.

p.s. the same problem is also described in more detail here: https://symfit.readthedocs.io/en/stable/fitting_types.html#minimize-maximize