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

Fitting two peaks should return params in a well-defined order #302

Open
jvavrek opened this issue Dec 14, 2021 · 5 comments
Open

Fitting two peaks should return params in a well-defined order #302

jvavrek opened this issue Dec 14, 2021 · 5 comments

Comments

@jvavrek
Copy link
Contributor

jvavrek commented Dec 14, 2021

I'm currently getting something like

>>> fitter.best_values

{...
'gauss1_mu': 1345,
...
'gauss0_mu': 1461
}

We should use parameter names in ascending order of energy:

>>> fitter.best_values

{...
'gauss0_mu': 1345,
...
'gauss1_mu': 1461
}
@markbandstra
Copy link
Member

Do you think this sorting is best done after the fitting has been performed (e.g., renaming the parameter names based on their order), or encoded in the parameter constraints before fitting? For the latter the documentation says we would have to introduce an auxiliary variable, like this:

model.add("delta_mu_01", min=5.0, vary=True)
model.set_param_hint("gauss1_mu", expr="gauss0_mu + delta_mu_01")

Maybe this could be done in Fitter._make_model?

@jvavrek
Copy link
Contributor Author

jvavrek commented Dec 14, 2021

Do you think this sorting is best done after the fitting has been performed (e.g., renaming the parameter names based on their order), or encoded in the parameter constraints before fitting? For the latter the documentation says we would have to introduce an auxiliary variable, like this:

model.add("delta_mu_01", min=5.0, vary=True)
model.set_param_hint("gauss1_mu", expr="gauss0_mu + delta_mu_01")

Maybe this could be done in Fitter._make_model?

Renaming after the fit was the first thing I thought of, but I imagine that could break all kinds of internal data. Probably best to introduce that auxiliary variable. Nice find!

@markbandstra
Copy link
Member

Renaming after the fit was the first thing I thought of, but I imagine that could break all kinds of internal data. Probably best to introduce that auxiliary variable. Nice find!

If that approach works well, I think it would be a cleaner way to do it -- e.g., (I don't know for sure) but renaming could mess with the covariance matrix. I guess we could check for multiple gauss*_mu and add some logic (in case there are three). Are there other model types we would want to sort like this?

@jvavrek
Copy link
Contributor Author

jvavrek commented Dec 14, 2021

If that approach works well, I think it would be a cleaner way to do it -- e.g., (I don't know for sure) but renaming could mess with the covariance matrix. I guess we could check for multiple gauss*_mu and add some logic (in case there are three). Are there other model types we would want to sort like this?

We should probably handle multiple expgauss as well. Or any other peak variants, or combinations thereof (e.g. one "gauss", one "expgauss").

@jvavrek
Copy link
Contributor Author

jvavrek commented Dec 15, 2021

In the meantime, one can use the limits kwarg to specify non-overlapping ranges, at least with backend="iminuit-pml":

fitter.fit(
    backend="iminuit-pml",
    limits={
        "gauss0_mu": (1300, 1400),
        "gauss1_mu": (1500, 1600),
    }
)

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

No branches or pull requests

2 participants