Skip to content

LinearLSQFitter cannot fit compound models #6038

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

Open
jehturner opened this issue May 10, 2017 · 5 comments
Open

LinearLSQFitter cannot fit compound models #6038

jehturner opened this issue May 10, 2017 · 5 comments
Labels
Docs Effort-low good first issue Issues that are well-suited for new contributors modeling Package-novice

Comments

@jehturner
Copy link
Member

jehturner commented May 10, 2017

Undo the doc modifications in #6041 before closing this as resolved.

@nden tells me that "I think in general a compound model should be fitted with a non-linear fitter because at least currently the LinearFitter cannot construct the Vandermonde matrix for compound models". It would be useful to have this working if possible, as I believe non-linear fitters are not guaranteed to converge on the solution without a good starting guess, which may not always be available. Also, the documentation should mention that it currently doesn't work (which I'll fix in the meantime). This is just for the record and in case someone wants to work on it before one of us gets to it.

Running LinearLSQFitter on a couple of compound models currently gives errors like the following:

>>> LinearLSQFitter()(Mapping((0,)) | Chebyshev1D(2), range(0,10), range(0,10))
Traceback (innermost last):
  File "<console>", line 1, in <module>
  File "/home/jturner/anaconda2/envs/peylian/lib/python2.7/site-packages/astropy-2.0.dev18162-py2.7-linux-x86_64.egg/astropy/modeling/fitting.py", line 254, in __call__
    raise ModelLinearityError('Model is not linear in parameters, '
ModelLinearityError: Model is not linear in parameters, linear fit methods should not be used.
>>> LinearLSQFitter()(Chebyshev1D(2) | Chebyshev1D(2), range(0,10), range(0,10)) 
Traceback (innermost last):
  File "<console>", line 1, in <module>
  File "/home/jturner/anaconda2/envs/peylian/lib/python2.7/site-packages/astropy-2.0.dev18162-py2.7-linux-x86_64.egg/astropy/modeling/fitting.py", line 279, in __call__
    lhs = model_copy.fit_deriv(x, *model_copy.parameters)
TypeError: 'NoneType' object is not callable
@jehturner
Copy link
Member Author

@pllim: does "Effort-low" refer to updating the docs or to constructing a combined Vandermonde matrix?

@pllim
Copy link
Member

pllim commented May 10, 2017

The docs 😄

@jehturner
Copy link
Member Author

Thanks. I was wondering if I should leave this for some new contributor at tomorrow's sprints, but there are lots of "effort-low/package-novice" issues, so I'll just do the docs (which could conceivably help people sprinting) 😄.

@pllim
Copy link
Member

pllim commented May 10, 2017

Update: First failure addressed in #6018 and now produces the same error as the second one.

@jehturner
Copy link
Member Author

Just a thought for later: For this to work, I believe we need a reliable way of determining whether a compound model is linear after combining its constituent models, which may or may not be the case. There is some such logic in _CompoundModelMeta, but, at a glance, it seems quite simplistic and I think the true outcome will depend on the individual models, as well as on the operator. I'm not sure whether this is feasible to do rigorously with a reasonable effort, as alluded to by an existing comment. Currently, however, the | operator produces linear=True when both operands are linear, which doesn't seem like it can be generally correct (since the parameters of the first model may get coupled to multiple terms of the second one)?

rkiman added a commit to rkiman/astropy that referenced this issue Mar 22, 2018
As mentioned in the issue astropy#6038, the LinearLSQFitter currently does not handle compound models. @jehturner added a note about that in the narrative docs. I'm extending that note to the docstrings in order to make this fact even clearer for users.
@pllim pllim added the good first issue Issues that are well-suited for new contributors label May 23, 2018
@astrofrog astrofrog added sprint and removed sprint labels May 23, 2018
astromancer pushed a commit to astromancer/astropy that referenced this issue Sep 18, 2018
As mentioned in the issue astropy#6038, the LinearLSQFitter currently does not handle compound models. @jehturner added a note about that in the narrative docs. I'm extending that note to the docstrings in order to make this fact even clearer for users.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Docs Effort-low good first issue Issues that are well-suited for new contributors modeling Package-novice
Projects
None yet
Development

No branches or pull requests

3 participants