WIP: Improve modeling docs how to create user-defined models and fitters#1878
WIP: Improve modeling docs how to create user-defined models and fitters#1878cdeil wants to merge 8 commits into
Conversation
There was a problem hiding this comment.
Since this uses scipy.optimize.fmin_slsqp then in principle all of these constraint types should be supported. Though at first glance it doesn't look like the current implementation of this fitter is actually supporting them.
There was a problem hiding this comment.
SLSQPFitter supports all these constraints. However, for clarity of the example, and because "ineqcons" and "eqcons" are specific to SLSQP and have not been tested much, I think we can exclude them.
Also it may be good to add a comment that the call to _model_to_fit_params() is needed in order to deal with "fixed" and "tied" constraints. It also handles "bounds" for optimization algorithms which (unlike SLSQP) don't internally handle it. Because "fmin_slsqp" works internally with bounds, they must be passed to it in the correct form (see comment below).
|
Do we want the |
|
I agree these need more attention. 👍 on splitting the examples for models and fitters and 👍 on extracting the examples into python files. For the content, I think for models we need an example of using We've talked before about separating optimization algorithms and fit statistic in fitters and I think we should aim to get this included in 0.4. I will make a separate issue about this but I think for 0.3.1 we should just correct the existing example and write a more complete example for 0.4 showing how to combine statistic and optimization methods to create a fitter. I like how you started So to summarize how I see this: for 0.3.1:
for 0.4
Comments about this plan? @cdeil I'll try to review this PR today. |
|
How can I avoid this error (i.e. make this optional if |
|
@nden Agree with everything you say. So this PR would be the modeling docs fixes for 0.3.1 and we'll discuss the 0.4 changes elsewhere. I'll wait for your review and then try to make all changes at once. Can't resist ... some comments on 0.4:
|
|
As for |
There was a problem hiding this comment.
As mentioned above SLSQP handles internally bounds and they must be passed to the minimizer in the correct form:
pars = [getattr(model_copy, name) for name in model_copy.param_names]
bounds = [par.bounds for par in pars if par.fixed != True and par.tied == False]
bounds = np.asarray(bounds)
for i in bounds:
if i[0] is None:
i[0] = DEFAULT_MIN_BOUND
if i[1] is None:
i[1] = DEFAULT_MAX_BOUND
|
@cdeil The models examples look good to me. Does adding |
|
Puh, it's hard to come back to old pull requests after a few months ... sorry for dropping the ball on this PR. |
|
@nden @embray The |
|
What is the status of this? |
|
Well, @cdeil would have to rebase this before going anywhere with it. But it's probably not worth it at the moment. |
|
@embray I'm closing this ... by now it would probably be better to re-start from |
|
I would like to leave it open for now merely so that I don't forget about it, as there are some good examples in it that I might like to use later. I will forget this exists otherwise. I'll assign it to myself so you don't have to worry about it. |
|
Removing the milestone since it isn't intended for merging. |
|
It's been another two years. |
This pull request tries to improve the sections of the modeling docs that explain how to implement user-defined models and fitters.
@nden @embray @astrofrog Please review.
TODO:
TODOmarkers with actual line numbers after review.new_model.rstin the same style I rewrotenew_fitter.rst.