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
This used to work, #1086 broke it, #1680 partially fixed it but introduced new behaviour, namely this is now allowed:
model = MyModel(param1=[[1,2], [2,3]], param2=100, param_dim=2)
In the second example the value of pram2 is repeated param_dim times.
Any reason not to have the first example allowed (it raises an error)?
Part of the problem is that Parameter.__set__ is used to set the initial value of a parameter as well as to update a parameter value. For the second use case it needs to verify that the shape of the new value is the same as the shape of the old one. But it does this verification in the first use case as well, before the shape has been determined.
@embray I don't think it is unreasonable to have a parameter of type array or any iterable. An example is a model which takes a rotation matrix and a shift. There are other examples.
Looking at this again, I agree that the first case:
model = MyModel(param1=np.array([[1,2], [3.,4]]), param2= 4.5, param_dim=1)
should work. The problem is the way values like param1=np.array([[1, 2], [3, 4]]) previously worked, in particular with regard to fitting. This effectively treats all 4 elements of param1 as individually fittable parameters, which is fine. That makes perfect sense.
But then even if you do
model = MyModel(param1=np.array([[1,2], [3.,4]]), param2=[4.5, 4.5], param_dim=2)
Now the same value passed in to param1 is interpreted completely differently: It's two param sets each with a 1x2 value for param1. I guess I've always found it confusing that param_dim determines the shapes of individual parameter values. In particular since each parameter can have values of different or even arbitrary "dimensions".
@embray, @nden Do you have a solution for this issue?
(I'm not using parameter arrays myself, so I have no clue / opinion here, but I understand it's important for your applications and needs to be resolved so that we can move on with other work in astropy.modeling well before the 0.4 release, e.g. #1827 or #1914)
Activity
embray commentedon Feb 5, 2014
This is all a little abstract. Maybe it would be better to discuss in terms of actual use cases than what the API allows...?
nden commentedon Feb 6, 2014
@embray I don't think it is unreasonable to have a parameter of type array or any iterable. An example is a model which takes a rotation matrix and a shift. There are other examples.
embray commentedon Feb 6, 2014
Oh, I agree there. I think the problem before was that there were ambiguities involved...
embray commentedon Feb 10, 2014
Looking at this again, I agree that the first case:
should work. The problem is the way values like
param1=np.array([[1, 2], [3, 4]])previously worked, in particular with regard to fitting. This effectively treats all 4 elements ofparam1as individually fittable parameters, which is fine. That makes perfect sense.But then even if you do
Now the same value passed in to
param1is interpreted completely differently: It's two param sets each with a 1x2 value for param1. I guess I've always found it confusing thatparam_dimdetermines the shapes of individual parameter values. In particular since each parameter can have values of different or even arbitrary "dimensions".cdeil commentedon Feb 19, 2014
@embray, @nden Do you have a solution for this issue?
(I'm not using parameter arrays myself, so I have no clue / opinion here, but I understand it's important for your applications and needs to be resolved so that we can move on with other work in
astropy.modelingwell before the 0.4 release, e.g. #1827 or #1914)astrofrog commentedon Jun 26, 2014
@embray - can this be closed now that #2634 has been merged?
embray commentedon Jun 26, 2014
Resolved, for now, by #2634.