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

Fit with custom parametric model fails #128

Closed
stestoll opened this issue Mar 29, 2021 · 2 comments · Fixed by #129
Closed

Fit with custom parametric model fails #128

stestoll opened this issue Mar 29, 2021 · 2 comments · Fixed by #129
Milestone

Comments

@stestoll
Copy link
Collaborator

import deerlab as dl
import numpy as np
import matplotlib.pyplot as plt

t = np.linspace(-0.2,3,201)
r = np.linspace(1.5,6,201)
K0 = dl.dipolarkernel(t,r)

def model(par):
    center, width, lam, conc, amp = par
    B = dl.bg_hom3d(t,conc)
    P = dl.dd_gauss(r,[center, width])
    V = amp*((1-lam)+lam*K0@P)*B
    return V

parref = [3.5,0.2,0.4,100,10]
Vref = model(parref)
V = Vref + dl.whitegaussnoise(t,np.max(Vref)*0.02)

par0 = parref
lb = [2, 0.05, 0.01,  10, 1]
ub = [5,   1,    1, 1000, 30]
fit = dl.fitparamodel(V,model,par0,lb,ub)

gives

c:\DeerLab\deerlab\fitparamodel.py:223: UserWarning: The fitted value of parameter #4, is at the lower bound (1.0).
  warnings.warn('The fitted value of parameter #{}, is at the lower bound ({}).'.format(p,lb[p]))
@stestoll stestoll added the bug Something isn't working label Mar 29, 2021
@stestoll stestoll added this to the 0.13.0 milestone Mar 29, 2021
@luisfabib
Copy link
Member

This is because the scale is being fitted as well automatically, but you are including an amplitude parameter anyway. If one disables the automatic fit of the scale, the fit works

fit = dl.fitparamodel(V,model,par0,lb,ub,rescale=False)

So there is not really a bug in the function.

@stestoll stestoll removed the bug Something isn't working label Mar 29, 2021
@stestoll
Copy link
Collaborator Author

stestoll commented Mar 29, 2021

I see, a user error from my side!

What about renaming the rescale keword parameter to fitscale or autoscale? These names would be more consistent with its function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants