-
Notifications
You must be signed in to change notification settings - Fork 6
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
Fix scalar @system by converting arguments to matrices #175
Conversation
I like the idea in this PR 👍 My impression is that handling scalars in NB: I just fixed the conflicts wrt |
Yes! We did not consider that as a result of having both scalar and matrices there occur corner cases. Therefore, it simplifies if we convert everything to an MathematicalSystems.jl/src/macros.jl Line 412 in d71a284
|
On a different topic. Can I get some help with the doctest? 🙈 |
Oh yes, I also noticed that at some point. I think the reason is that we do not always write Here is an example from the doctest errors: │ extract_sum([:(A1*x)], :x, :u, :w)
│
│ Evaluated output:
│
│ 1-element Array{Tuple{Any,Symbol},1}:
│ (:(hcat(A1)), :A)
│
│ Expected output:
│
│ 1-element Array{Tuple{Any,Symbol},1}:
│ (:(hcat(:A1)), :A) Here there is a colon ( Also notice that when @mforets merged the latest changes here, other tests started breaking. |
Thanks a lot. I did not notice it! |
Thinking about other solutions to handle scalar system in
@system
.The problems with the constructor for scalar systems could be fixed by converting the argument with
hcat
andvcat
to amatrix
respectively avector
.Basically, the same we did in the constructor for
Number
arguments.Which means, that from the point of the
@system
macro, the scalar constructor would not be needed.