-
-
Notifications
You must be signed in to change notification settings - Fork 206
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
Symbolic defaults for sub-components and base system kwargs + set base sys kwargs as component's kwargs #2226
Symbolic defaults for sub-components and base system kwargs + set base sys kwargs as component's kwargs #2226
Conversation
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.
AI-Maintainer Review for PR - Symbolic defaults for sub-components and base system kwargs + set base sys kwargs as component's kwargs
Title and Description 👍
Scope of Changes 👍
Documentation and Comments 👍
Testing 👎
Suggested Changes
- Please provide details on how the changes were tested. This could include the specific test cases used to verify the changes and any testing frameworks used.
Reviewed with AI Maintainer
e1f2b4d
to
49db726
Compare
|
The Ai4EComponentLib, StructuralIdentifiability integration test failures are unrelated to the changes introduced here. MTK test failure and MSL integration test failure is fixed by SciML/ModelingToolkitStandardLibrary.jl#209. However that needs this to be merged and released. This is ready for review @YingboMa @ChrisRackauckas. |
In your first example, could the default value for B(; b1 = a1^2) ? Thanks for this PR, I have been missing this feature :) |
Expressions without conditional part work. (I'm working on the latter in a follow up PR) So julia> @named aa = A(; a1 = 1)
Model aa with 0 equations
States (0):
Parameters (2):
a1 [defaults to 1]
bb₊b1 [defaults to a1^2]
julia> getdefault(aa.bb.b1)
a1^2 Something like |
8763cd0
to
a7fc669
Compare
Additional to above changes, a new
@YingboMa I've updated the tests to include this and rebased on current master. So this PR is ready for review. |
5d6e27f
to
4e2549c
Compare
|
… of depending on the numerical value of it - this change allows user to set a parameter with initial value and pass that as default value of kwargs of sub components.
…rgs of base sys as components kwargs
4e2549c
to
c62f84f
Compare
- This will allow to provide arguments that aren't parameters/variables like int, function... - Adds tests for the same
c62f84f
to
a9fa244
Compare
The branch is rebased wrt to master.
*To test locally use |
extend
) can now have symbolic defaults. This allows scenarios like:Earlier, this would be
A(; a1 = nothing, B.b1 = a1)
, where b1 would have default value set as a1 arguement.Now, it is
A(; a1 = nothing, B.b1 = nothing)
and internally, default value of b1 is set to parameter a1.Earlier D would have a kwarg
ee__ek
(as ifee
was a subcomponent). Now D will haveek
as the kwarg. This avoids having to access partial components of a model to set parameters.The model-parsing tests are updated to cover both the above changes.