New models, model testing and Parametric2DModel base class #1251
Conversation
|
@nden A short question: Is it currently possible to specify analytical derivatives for 2D models? How does the fitter handle this? I've made a short try and it didn't work... |
|
@adonath I thought so. Although it was also possible to fit polynomials with the |
|
I think the switch to column based derivative caused the regression with polynomials. We want to be able to fit linear models with non-linear fitters. |
|
OK, once I fix the derivative of |
|
@nden Concerning the 2D derivatives: Concerning the Polynomials: What kind of example do you need? E.g. there is the Gaussian1DModel which is an nonlinear Model with derivatives. A 2D model with dervivatives doesn't exist yet. But I have one example in another branch. Should I add it to this pull request? Thanks for the comments! |
|
@adonath I'm working on something else, so please add the fix for polynomials to this PR. (1) and (2) is what I did to get it working yesterday as well but I didn't do it consistently. I was asking if you have an example of a 2D non-linear model with derivatives which I can use for debugging but since it works, that's fine, no need to post one now. How do you want to proceed with #1243 ? I was thinking we can merge this one first and then #1243 but I'm not sure how much other work you are doing. Would it be better to wait even longer? |
|
I think this PR is almost finished. I will fix the polynomials and extend the documentation a bit (add some Latex formulae and "See Also" sections in the docstrings etc.). I will also add a new BetaModel. But this will be done by the end of the week. |
|
I think this PR is finished. @nden Would you take a look at the changes? |
| from .. import models | ||
| from ..core import * | ||
| import numpy as np | ||
| from numpy.testing import utils | ||
| from ...tests.helper import pytest | ||
| from .. import fitting | ||
| from .model_lists import models_1D, models_2D | ||
| from astropy.modeling.core import Parametric1DModel, Parametric2DModel | ||
| from astropy.modeling.polynomial import PolynomialModel |
There was a problem hiding this comment.
Minor: these should be relative imports
|
Apart from a couple of minor comments, this looks good to me. |
|
@adonath - why did you close this pull request? EDIT: I'm re-opening this since I'll assume this was accidental |
|
Thanks for reopening! I don't know why it was closed... |
|
No problem! @nden - I think this is ready for a final review! |
| @@ -19,6 +19,7 @@ | |||
| from numpy import linalg | |||
| from ..logger import log | |||
| from .utils import poly_map_domain | |||
| from .polynomial import PolynomialModel | |||
There was a problem hiding this comment.
@adonath I was hoping to not have any models related imports in the fitting module and keep models and fitting completely separate. One reason is that in the future this may introduce more model specific dependencies. So would it make more sense to make col_deriv an attribute of ParametricModel with a default value of 1 and set to 0 in PolynomialModel and OrthogPolyBase? Then a fitter would use model.col_deriv.
There was a problem hiding this comment.
This makes totally sense to me. I will change this, as you proposed...
|
I think this is great. After the |
|
@nden - can you confirm whether this looks ok to you now? |
|
@astrofrog Looks good to merge. |
New models, model testing and Parametric2DModel base class
|
Thanks @adonath! |
|
And thanks @nden for reviewing this :) |
This is a follow up PR to #1203.
At first I've implemented a new Parametric2DModel class, which is the base class for all 2D Models.
Than I've coded a few more 1D and 2D models. Including Box, Trapezoid, MexicanHat, mainly models
which are needed for the convolution kernel class (I will open a second PR for the convolution kernel class today...)
To have proper testing for all of these models, I've restructured "test_model.py" a bit. The main difference is, that
there is an additional file "model_lists.py" in the test folder, where all the models test values and test parameters are coded in a dictionary (this is more readable than simple lists in between the code and I assume it will get much longer in the future). Every model in this list is automatically tested for returning the right values, when called and for working with the NonLinearLSQFitter.
To make implementing new models still easier, I've written an IPython notebook, where you can calculate model
test values and derivatives with SymPy (here is a link). It also outputs a Latex string for the model docstring and shows
the model formula with symbols, which is a great test, to see if the model formula is correct. I think it would
be useful for everyone, who wants to implement new models. What would be the best place to put this notebook?
Currently it is just in the docs/modeling folder.
The work is "almost finished": Some models are only tested with one single value and most Latex model formulae
in the docstrings are missing. But this can be easily done with the Notebook.
I'm looking forward to any comments!