Skip to content
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

Implement substitute for AbstractSystems #2257

Merged
merged 3 commits into from
Sep 14, 2023
Merged

Implement substitute for AbstractSystems #2257

merged 3 commits into from
Sep 14, 2023

Conversation

YingboMa
Copy link
Member

No description provided.

@baggepinnen
Copy link
Contributor

baggepinnen commented Sep 13, 2023

How would this work if I want to substitute a subsystem that is deeper than level one? For example, if I want to substitute the following component in the DC motor example?

addPI = Blocks.Product() # My new component
substitute(model, Dict(pi_controller.addPI => addPI))

@YingboMa
Copy link
Member Author

Just :addPI => addPI because it's recursive.

@baggepinnen
Copy link
Contributor

So if there's more than one PI controller in the model, it will change all of them? What if I only want to change one of them?

@YingboMa
Copy link
Member Author

This function is not created for that use case.

@baggepinnen
Copy link
Contributor

In that case, there should probably by something more than just name that identifies the components to substitute since using the name alone could easily lead to problems if the replaced name is used more than once, for potentially different component types, by multiple different component authors.

@ChrisRackauckas
Copy link
Member

The name cannot be used more than once, and component authors don't specify the name. They are set when defining the instance (via @named) and its local to the user. Indeed if there's name collusions we should (and do?) error.

I think what you're getting at though is that we should have a form of typing on components, so we can identify all of the components that are the same type. That's a wholly separate thing though which would be nice to have for other reasons as well.

@baggepinnen
Copy link
Contributor

and component authors don't specify the name

The substitute function operates recursively, so as I understand it it considers the un-namespaced name of every single subsystem in present in the model?

@ChrisRackauckas
Copy link
Member

That's a good point to clarify.

@YingboMa
Copy link
Member Author

Yes, those are valid concerns, but they are just not what this function does. substitute works recursively, and if you want to only replace one specific model in a specific level, we need to implement a different function.

@YingboMa YingboMa merged commit da5fa87 into master Sep 14, 2023
28 of 33 checks passed
@YingboMa YingboMa deleted the myb/sub_sys branch September 14, 2023 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants