-
Notifications
You must be signed in to change notification settings - Fork 168
Array variable different from array of variables #379
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
Comments
No, removing array variables is 100% and unequivocally the wrong direction. Yes, array variables are hard. Yes, they are incomplete. No, we should not give up. Array variables allow for O(1) representations of things like matrix multiplication. If you want to represent modern algorithms like neural networks symbolically, you need them. But then you need vector calculus, and you need vector codegen, etc. Instead of shying away from the problem, we are saying that this is one of the big problems we need to tackle. It's part of our longer term scope. Until then, yes the easiest way to make arrays easier is to just scalarize them. |
Sorry, I don't understand all the terminology, but I didn't think I was suggesting this. All I want is for |
They should behave in the same ways, but they should not be the same. Your approach |
To be clear, I was not at all suggesting that this is desirable; quite the opposite — hence the issue. I just used it as a proof of concept to demonstrate what was going wrong, and as a temporary workaround. I object to this being referred to as my approach. :)
So I guess I'm suggesting that there was a recent regression, because #370, #371, #372, and #376 were the only examples I could find where this was coming up. |
Array variables were added in August in the breaking v1.0 release. |
I think downstream code should as much as possible not use the types |
Agreed. I think one issue is |
There's interface for that now as well. |
Nowadays, the example I gave above returns julia> typeof(v1.val)
SymbolicUtils.BasicSymbolic{Real}
julia> typeof(v[1].val)
SymbolicUtils.BasicSymbolic{Real} and the issues I mentioned above seem to be fixed (or failing for different reasons). |
Uh oh!
There was an error while loading. Please reload this page.
I think I'm not alone in expecting that indexing one element of an array variable should produce something that behaves the same as an ordinary (non-array) variable. For example, if I create
then I expect
v1
andv[1]
to behave the same. They don't, and this causes some errors.The problem is that, while
v1
andv[1]
are bothNum
s, they're different on the inside. Evidently,Num
is just a thin wrapper around theval
field, and that field contains different things in these two cases:Note that one is a
Sym
, and the other is aTerm
. I guessReal
is being broadcast across the elements ofv
or something??? (I don't pretend to understand howSymbolics
and friends work.)This seems to be the origin of several recent bugs: #370, #371, #372, and #376. In each of those cases, if I replace expressions like
v[1:N]
withv=[Symbolics.variable(:v, i) for i in 1:N]
, the problems go away. I guess you could argue that all downstream functions should be changed to accept bothSym
andTerm
from here on out, but I feel like it might be better to just change the behavior of@variables v[1:2]
.┆Issue is synchronized with this Trello card by Unito
The text was updated successfully, but these errors were encountered: