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

iminuit integration broken with iminuit==2.0.0 #291

Closed
kosiokarchev opened this issue Dec 11, 2020 · 3 comments
Closed

iminuit integration broken with iminuit==2.0.0 #291

kosiokarchev opened this issue Dec 11, 2020 · 3 comments

Comments

@kosiokarchev
Copy link

Hello,
A few days ago the iminuit package was updated to version 2.0.0, and now it seems that the integration with the fit_lc function is broken:

>>> import sncosmo
>>> data = sncosmo.load_example_data()
>>> model = sncosmo.Model(source='SALT2')
>>> sncosmo.fit_lc(data, model, ('t0',))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python3.9/site-packages/sncosmo/fitting.py", line 670, in fit_lc
    m = iminuit.Minuit(fitchisq, errordef=1.,
  File "/usr/lib/python3.9/site-packages/iminuit/_minuit.py", line 485, in __init__
    self._init_state = _make_init_state(self._pos2var, args, kwds)
  File "/usr/lib/python3.9/site-packages/iminuit/_minuit.py", line 1401, in _make_init_state
    raise RuntimeError(
RuntimeError: errordef is not one of the parameters []

This works when iminuit is downgraded to 1.5.4 (the latest 1.x version).

PS Thanks for your work on the package, it's a huge relief for someone just getting into the field!

@kboone
Copy link
Member

kboone commented Dec 12, 2020

It looks like the entire API was reworked in the latest version of iminuit. The changes are listed here: https://iminuit.readthedocs.io/en/stable/changelog.html

There will need to be some significant changes in sncosmo to get things working with iminuit 2.0. For now, I would recommend sticking with iminuit v1.5.4.

@kbarbary, how do you feel about this? With all of the changes, we'll need to rewrite a good chunk of fit_lc. It looks like that should be pretty straightforward, but it won't be easy to maintain compatibility with both iminuit v1.x and v2.x. We probably need to pick one of the two going forward...

@kboone
Copy link
Member

kboone commented Jan 29, 2021

I updated sncosmo to use the new iminuit API in #292, so checkout that branch if you want to use sncosmo with iminuit>=2.0.

@kboone
Copy link
Member

kboone commented Feb 9, 2021

#292 has been merged, so this is fixed in the master branch now.

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