You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
An example of a composite model would be e.g. Gaussian source plus another Gaussian source plus polynomial background, right?
Is it true that at the moment such additive models are not possible with astropy.modeling?
Being able to fit such a "signal plus background" model is pretty fundamental. @nden Can you briefly outline how you think it should be implemented so that we can see if it's possible for 0.3?
The main issue here is how to handle parameters of composite models.
PR #1985 implements parameters of composite models in the following way:
Parameters of composite models are descriptors which point to the parameters of the children models. The names of the parameters are a concatenation of the child model parameter name, the name of the model and the index of the model in the list of models. For example the names of the parameters of a composite model consisting of two Gaussian1D models are:
_CompositeModel implements also param_sets and parameters. parameters is also implemented as a descriptor which returns the stack of the parameters of all models. Its __set__ method sets the single parameters on the children models.
Note that currently parameters patches the class object while the single parameters instead are sort of a hack on the current parameter scheme. This has several consequences:
Tab expansion on a composite model object does not show the names of the parameters (I'd prefer to see them.)
Because the .parameter is attached to the class, tab expansion on a composite model object shows it. (How bad is patching like this?) Also, there is no ._parameters attribute, instead parameters is always computed.
One thing that currently does not work is indexing/slicing the .parameters array.
Any comments on the names of parameters?
Any ideas for improving this scheme?
Instead of this complicated nested parameter naming scheme, what if composite models were treated as indexable collections of models? So instead of model.amplitude_Gaussian1D_0 and model.amplitude_Gaussian1D_1 one can just do model[0].amplitude and model[1].amplitude respectively.
This does mean setting things like the parameter constraints is a little more complicated. But still not that bad. For example if we want to set model.fixed currently we assign it a dict mapping parameter names to the value of that constraint, like:
model.fixed = {'amplitude': False}
For a composite model this would have to use nested dicts. For example:
But we can go one better than that. Perhaps we should allow individual model instances to have a "name" attribute that can be assigned when the model is initialized:
g1 = Gaussian1D([...], name='gaussian1')
Then in addition to (or perhaps instead of) using numerical indices one can refer to components of the model by name as well:
composite_model['gaussian1'].amplitude
for example. This would work just as well for nested composite models as well. This would also work for setting constraints:
If the model is not given a name, one could be generated. This might as well use a schema similar to the one already proposed, like Gaussian1D_0. But I think that's also a good argument for allowing composite models to be indexed with integers as well. Admittedly I haven't fully considered the issues with this so there could be something I'm missing for why this wouldn't work...?
You can use the parameters of the individual models now. composite_model.transforms[0].amplitude=1 works. This is still not an indexable model (and at some point I had the composites as ordered dict) but it is closer to what you suggest.
The reason I have parameter names at the composite model level is to manage constraints easier, for example:
The purpose of this PR is to hear opinions on the API especially from people who use constraint fitting in their work (I am not one of them). In my limited use of constraint fitting in other packages nested constraints are a bit confusing.
So if I take as an example 3 models model = poly1 * (gauss1 + gauss2), I would like to hear preferences about:
How to access individual model parameters.
Do we need parameters at the composite model level, like
As a programmer I find those long generated attribute names to be very jarring and difficult to look at, especially when I know that what it represents is an actual data structure that I can drill down into more "naturally" to get to the individual parameters I want to tweak. But that's just me--I don't know what it looks like to a non-programmer. I think either way could have advantages or disadvantages (though the way I suggested would also be aided by a really good __repr__ for nested models that makes it clear how to drill down into them).
I'm also not using constraints (other than min / max for individual parameters) in my work, so I can't comment on what would be best for those users.
But @adonath and I do use composite models in Sherpa extensively in an analysis pipeline, so I'll make some comments:
👍 to attach names to model instances.
This helps in interactive work and for storing fit results (for which we use JSON).
👎 on using underscores (_) to flatten the hierarchical model parameter names to a into a simple flat list of parameters names. If this is needed at all, I would suggest to use the dot (.) instead, because _ can be part of model and parameter names and thus it will not be clear how many levels there are and where they start / stop.
I haven't thought about this much or have time to investigate, but playing around with Sherpa a bit could be helpful ... if I understand correctly they do not implement the flat parameter names like "mean_Gaussian1D_1_SummedCompositeModel_0" as you propose above (but don't support complex constraints like astropy.models does either): https://gist.github.com/cdeil/9308117
Maybe raise this question on the list to get some more feedback before deciding?
Activity
cdeil commentedon Aug 7, 2013
An example of a composite model would be e.g. Gaussian source plus another Gaussian source plus polynomial background, right?
Is it true that at the moment such additive models are not possible with
astropy.modeling?Being able to fit such a "signal plus background" model is pretty fundamental.
@nden Can you briefly outline how you think it should be implemented so that we can see if it's possible for 0.3?
nden commentedon Aug 7, 2013
@cdeil I am currently working on this, hope to post it for initial review soon.
But yes, your example is correct.
astrofrog commentedon Sep 19, 2013
@nden - just to check, is this something that is still in development?
perrygreenfield commentedon Sep 19, 2013
she is out of the country for another week (and has been for most of the month)
embray commentedon Sep 20, 2013
As far as I know yes--we were talking about it a bit before she left.
Assuming the distortion models return the deltas, i.e. the result of …
nden commentedon Jan 21, 2014
The main issue here is how to handle parameters of composite models.
PR #1985 implements parameters of composite models in the following way:
Parameters of composite models are descriptors which point to the parameters of the children models. The names of the parameters are a concatenation of the child model parameter name, the name of the model and the index of the model in the list of models. For example the names of the parameters of a composite model consisting of two Gaussian1D models are:
['amplitude_Gaussian1D_0', 'mean_Gaussian1D_0', 'stddev_Gaussian1D_0', 'amplitude_Gaussian1D_1', 'mean_Gaussian1D_1', 'stddev_Gaussian1D_1']
This scheme works with nested models as well. If the above model is combined with a Polynomial1D model, the names are:
['amplitude_Gaussian1D_0_SummedCompositeModel_0', 'mean_Gaussian1D_0_SummedCompositeModel_0', 'stddev_Gaussian1D_0_SummedCompositeModel_0', 'amplitude_Gaussian1D_1_SummedCompositeModel_0', 'mean_Gaussian1D_1_SummedCompositeModel_0', 'stddev_Gaussian1D_1_SummedCompositeModel_0', 'c0_Polynomial1D_1', 'c1_Polynomial1D_1']
_CompositeModelimplements alsoparam_setsandparameters.parametersis also implemented as a descriptor which returns the stack of the parameters of all models. Its__set__method sets the single parameters on the children models.Note that currently
parameterspatches the class object while the single parameters instead are sort of a hack on the current parameter scheme. This has several consequences:Tab expansion on a composite model object does not show the names of the parameters (I'd prefer to see them.)
Because the
.parameteris attached to the class, tab expansion on a composite model object shows it. (How bad is patching like this?) Also, there is no._parametersattribute, insteadparametersis always computed.One thing that currently does not work is indexing/slicing the
.parametersarray.Any comments on the names of parameters?
Any ideas for improving this scheme?
embray commentedon Feb 4, 2014
Instead of this complicated nested parameter naming scheme, what if composite models were treated as indexable collections of models? So instead of
model.amplitude_Gaussian1D_0andmodel.amplitude_Gaussian1D_1one can just domodel[0].amplitudeandmodel[1].amplituderespectively.This does mean setting things like the parameter constraints is a little more complicated. But still not that bad. For example if we want to set
model.fixedcurrently we assign it a dict mapping parameter names to the value of that constraint, like:For a composite model this would have to use nested dicts. For example:
But we can go one better than that. Perhaps we should allow individual model instances to have a "name" attribute that can be assigned when the model is initialized:
Then in addition to (or perhaps instead of) using numerical indices one can refer to components of the model by name as well:
for example. This would work just as well for nested composite models as well. This would also work for setting constraints:
If the model is not given a name, one could be generated. This might as well use a schema similar to the one already proposed, like
Gaussian1D_0. But I think that's also a good argument for allowing composite models to be indexed with integers as well. Admittedly I haven't fully considered the issues with this so there could be something I'm missing for why this wouldn't work...?nden commentedon Feb 5, 2014
You can use the parameters of the individual models now.
composite_model.transforms[0].amplitude=1works. This is still not an indexable model (and at some point I had the composites as ordered dict) but it is closer to what you suggest.The reason I have parameter names at the composite model level is to manage constraints easier, for example:
But I have no strong feelings about this.
The purpose of this PR is to hear opinions on the API especially from people who use constraint fitting in their work (I am not one of them). In my limited use of constraint fitting in other packages nested constraints are a bit confusing.
So if I take as an example 3 models
model = poly1 * (gauss1 + gauss2), I would like to hear preferences about:Do we need parameters at the composite model level, like
or is it sufficient to have
Any objections?
embray commentedon Feb 5, 2014
As a programmer I find those long generated attribute names to be very jarring and difficult to look at, especially when I know that what it represents is an actual data structure that I can drill down into more "naturally" to get to the individual parameters I want to tweak. But that's just me--I don't know what it looks like to a non-programmer. I think either way could have advantages or disadvantages (though the way I suggested would also be aided by a really good
__repr__for nested models that makes it clear how to drill down into them).cdeil commentedon Mar 2, 2014
I'm also not using constraints (other than min / max for individual parameters) in my work, so I can't comment on what would be best for those users.
But @adonath and I do use composite models in Sherpa extensively in an analysis pipeline, so I'll make some comments:
👍 to attach names to model instances.
This helps in interactive work and for storing fit results (for which we use JSON).
👎 on using underscores (
_) to flatten the hierarchical model parameter names to a into a simple flat list of parameters names. If this is needed at all, I would suggest to use the dot (.) instead, because_can be part of model and parameter names and thus it will not be clear how many levels there are and where they start / stop.I haven't thought about this much or have time to investigate, but playing around with Sherpa a bit could be helpful ... if I understand correctly they do not implement the flat parameter names like
"mean_Gaussian1D_1_SummedCompositeModel_0"as you propose above (but don't support complex constraints likeastropy.modelsdoes either):https://gist.github.com/cdeil/9308117
Maybe raise this question on the list to get some more feedback before deciding?
astrofrog commentedon May 8, 2014
Re-scheduling to 1.0
astrofrog commentedon Jan 14, 2015
@embray - can this be closed now that #3231 has been merged?
embray commentedon Jan 14, 2015
Superseded by #3231.