-
-
Notifications
You must be signed in to change notification settings - Fork 224
fix getvar
access of state
#1824
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
Conversation
The result of ```julia i = findfirst(x -> getname(x) == name, sts) ``` right above the added code was never used, causing some states not to be found. I'm no sure if the added code is correct, but something is at least missing here
depends on SciML/ModelingToolkit.jl#1824
depends on SciML/ModelingToolkit.jl#1824
depends on SciML/ModelingToolkit.jl#1824
@@ -304,6 +304,9 @@ function getvar(sys::AbstractSystem, name::Symbol; namespace = false) | |||
|
|||
sts = get_states(sys) |
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.
These are already handled above. We should just remove this block.
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 had an example that failed without this change but not with it. It's from the analysis point PR.
It might be related to how I added states to the system here?
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.
Ah, yeah, we should remember to update the var_to_name
map, when vars are updated.
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.
We should remove the later state index check.
Let's merge this for now, but we should work on a more efficient solution later. |
The result of
right above the added code was never used, causing some states not to be found. I'm no sure if the added code is correct, but something is at least missing here