-
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
Error handling for repeated variable names #164
Conversation
Co-Authored-By: Christian Schilling <[email protected]>
Co-Authored-By: Christian Schilling <[email protected]>
Co-Authored-By: Christian Schilling <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I accept but I do not think this is a good solution.
After thinking everything through more again more carefully, I think you are right. In order for me to understand what is going on, I like to write everything I know down As far as i know, this PR should give reasonable error messages for cases where A = B = D = rand(2,2)
@system(a⁺= Aa + Bw, input:a)
ERROR: ArgumentError: state and input variables have the same name `a`
# instead of the old error message
ERROR: ArgumentError: the entry (:A, :D) does not match a `MathematicalSystems.jl` structure
@system(x⁺= Ax + Bx, input:x)
ERROR: ArgumentError: state and input variables have the same name `x`
@system(x⁺= Ax + Bu + Bw, input:u, noise:u)
ERROR: ArgumentError: input and noise variables have the same name `u` without throwing exceptions where the input is "overwritten" with the noise variable # we can use
@system(x⁺= Ax + Bw, input:w) #creates a LinearControlDiscreteSystem system
system(x⁺= Ax + Bu, noise:u) #
# instead of
@system(x⁺= Ax + Bw, input:w, noisestr:evry_but_w) in the However, this cases are not covered so far in this PR (but we also should add it) @system(x⁺= Ax + Bw + Dw, input:w)
@system(x⁺= Ax + Bu + Bu, noise:u)
@system(x⁺= Ax + Bx)
@system(x⁺= Ax + c + c)
# etc which gives a non-sensical error messsage. (for which my aforementioned fix Hoooowever, going through this logic it should also give the right error for @system(x⁺= Ax + Bu + Bw, noise:u)
ERROR: ArgumentError: the entry (:A, :B, :c) does not match a `MathematicalSystems.jl` structure
@system(x⁺= Ax + Bu + Bw, input:w)
ERROR: ArgumentError: the entry (:A, :c, :B) does not match a `MathematicalSystems.jl` structure which it does not, and also will not get caught by my solutions. Sooo, too sum up, I am bit confused. I think we could do it better, you are right, EDIT: noo, it does not work to make the test later, because we will infer the wrong system types 🙈🤷♂️ |
Yes, that is what i thought (but i haven't tried it). 👍 i moved your comment to the follow up issue, #181 |
Thanks! But we still have to catch the error from MathematicalSystems.jl/src/macros.jl Line 807 in a1d94d6
|
From discussion on gitter, consider this PR in place of #155.