tBuLi / symfit

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

Fit does not warn when constraints are given with a minimizer that does not support them #266

Open bhclowers opened 5 years ago

bhclowers commented 5 years ago

Using symfit 0.5.1 I'm running in to what I imagine are similar issues. Essentially, I cannot get the constraints to be accepted and met. In short, I have two parameters that I'm trying to fit (theta1 and theta2) and I need the first theta to be larger than the second and have them both be greater than 0. However, running the following code I get these values for the fitted thetas:

Fitted Theta-1: 5286.70 Fitted Theta-2: -27.42

So the first issue appears to be that the constraints are being ignored and secondly, if you don't specify a minimizer when passing the constraints, the thetas are driving both to 1 (which is also incorrect). I looked at a few other threads but didn't find one that matched this particlar problem. Any thoughts/ideas would be appreciated.

import numpy as np
from symfit import Parameter, Variable, Fit, parameters, variables, Parameter, GreaterThan
from symfit.core.minimizers import *

#Non working input arrays
xdata = np.array([0.0, 6.93e-05, 0.000139, 0.000277, 0.000554, 0.00111, 0.00166, 0.00222])
ydata = np.array([1.0, 1.0038328859204988, 1.0191644062228622, 1.038328900011579, 1.0498275723826584, 1.0574932885739006, 1.0613261932627833, 1.0613261932627833])

#Beginning of Symfit Parameters
theta1, theta2 = parameters('theta1, theta2')
x, y = variables('x, y')

phi1 = 0.942697425415 #  Fixed Value
phi2 = 0.971320906503 # Fixed Value

model_dict = {y: (1+x*theta1+theta2*x**2)/(1+phi1*x*theta1+phi2*theta2*x**2)} #model to fit (Equation 1)
constraints = [GreaterThan(theta1, theta2), GreaterThan(theta2,0)] #Constraint to make sure that the first clustering event is dominant and all values are positive

#If you comment out the minimizer the fitted results for the thetas both equal 1 which is incorrect
fit = Fit(model_dict, x=xdata, y=ydata, constraints=constraints, minimizer = LBFGSB)# Optional Minimizer = LBFGSB Feed in the observed x and y data
fit_result = fit.execute()

print("Fitted Theta-1: %.2f"%fit_result.value(theta1))
print("Fitted Theta-2: %.2f"%fit_result.value(theta2))

print(fit_result)
pckroon commented 5 years ago

Hi,

first things first: LBFGSB does not support constraints (but you specified you want that minimizer, so we assume you know better). From the top of my head you can pick from SLSQP, COBYLA and TrustConstr. IIRC, by default Fit will take SLSQP. The resulting theta's being 1 means they haven't changed from their initial guesses. You can try with a better initial guess, or try one of the other algorithms. Next, it's (much) easier to specify theta >= 0 by setting theta1.min = theta2.min = 0 rather than using the constraint. Bounds are much easier from a minimization point of view that constraints. HTH!

pckroon commented 5 years ago

One more thing: would it not be possible to do the fit unconstrained, and sort the theta's by size afterwards?

bhclowers commented 5 years ago

Thanks for the direction. I think the fact that the values weren't changing while returning 1 was throwing me off. Even when reasonable first guesses were provided I "expected" some sore of refinement but didn't get it. I went through a range of minimizers and found that some worked better than others for this particular problem. I hope that my struggles will help others. In short, it is not obvious that the constraints are not accepted by certain minimizers as an error doesn't seem to be thrown and this combined with the odd behavior of the default minimizer was compounding my issues in understanding the results.

pckroon commented 5 years ago

I renamed the issue. To close this we would need to make Fit warn when constraints are given, and a minimizer was selected that does not support them. Same for bounds.

antonykamp commented 3 years ago

This feature could be added easily after merging #289, because it adds a helper function to collect subclasses eg. of ConstrainedMinimizer. Maybe this module should be moved with a PR of this issue :)