-
-
Notifications
You must be signed in to change notification settings - Fork 45
Adds permanent magnet DC machine #49
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
Codecov Report
@@ Coverage Diff @@
## main #49 +/- ##
==========================================
+ Coverage 68.03% 70.70% +2.66%
==========================================
Files 25 25
Lines 1214 1198 -16
==========================================
+ Hits 826 847 +21
+ Misses 388 351 -37
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
As a general principle, anything that you would not expect to be true for 99% of models should be required. So for example, if you know that in a given model everyone always uses the normalized units of |
This is doing a lot. Can this be rebased? Are there pieces that should be individual PRs? |
|
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.
It seems some of the components are not needed for DCPM. Does it make sense to split this PR a bit?
def61b3
to
1ac5485
Compare
dca6e94
to
42fff05
Compare
46d3144
to
be93225
Compare
sys = structural_simplify(model) | ||
prob = ODEProblem(sys, [], (0, 6.0)) | ||
sol = solve(prob, Rodas4()) | ||
|
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.
Using
prob = ODAEProblem(sys, [], (0, 6.0))
sol = solve(prob, Rodas4())
will result in
ERROR: UndefVarError: pi_controller₊err_input₊u(t) not defined
Stacktrace:
[1] macro expansion
@ .julia\dev\ModelingToolkit\src\structural_transformation\codegen.jl:199 [inlined]
[2] macro expansion
@ .julia\packages\SymbolicUtils\qulQp\src\code.jl:351 [inlined]
[3] macro expansion
@ C:\Users\kaisv.TUGRAZ\.julia\packages\RuntimeGeneratedFunctions\KrkGo\src\RuntimeGeneratedFunctions.jl:129 [inlined]
[4] macro expansion
@ .\none:0 [inlined]
[5] generated_callfunc
@ .\none:0 [inlined]
[6] (::RuntimeGeneratedFunctions.RuntimeGeneratedFunction{(Symbol("##arg#14840964602010972036"), Symbol("##arg#18305951688076727758"), :t), ModelingToolkit.StructuralTransformations.var"#_RGF_ModTag", ModelingToolkit.StructuralTransformations.var"#_RGF_ModTag", (0x8b4a2f6c, 0xa9522116, 0x134b5081, 0x0a8c04c2, 0x33f2532d)})(::Vector{Float64}, ::Vector{Float64}, ::Float64)
@ RuntimeGeneratedFunctions .julia\packages\RuntimeGeneratedFunctions\KrkGo\src\RuntimeGeneratedFunctions.jl:117
so that it can be represented as a system of `ODEs` (ordinary differential equations). | ||
|
||
```@example dc_motor_pi | ||
sys = structural_simplify(model) |
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.
Due to all Blocks.RealInputs
being marked as input=true
even the simplified system has a whopping 19 states:
julia> states(sys)
19-element Vector{Term{Real, Base.ImmutableDict{DataType, Any}}}:
pi_controller₊int₊x(t)
L1₊i(t)
inertia₊phi(t)
inertia₊w(t)
pi_controller₊gainPI₊input₊u(t)
pi_controller₊addPI₊input1₊u(t)
pi_controller₊addPI₊input2₊u(t)
pi_controller₊int₊input₊u(t)
pi_controller₊addTrack₊input1₊u(t)
pi_controller₊addTrack₊input2₊u(t)
pi_controller₊limiter₊input₊u(t)
pi_controller₊addSat₊input1₊u(t)
pi_controller₊addSat₊input2₊u(t)
pi_controller₊gainTrack₊input₊u(t)
feedback₊input2₊u(t)
feedback₊input1₊u(t)
source₊V₊u(t)
load₊tau₊u(t)
pi_controller₊err_input₊u(t)
9ea5de2
to
1523683
Compare
]) | ||
sys = structural_simplify(model) | ||
|
||
@test_broken prob = ODAEProblem(sys, Pair[], (0, 6.0)) |
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.
What does this fail with? Have you tried ODEProblem?
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.
The ODAEProbem
construction works fine but solve errors. ODEProblem
works. SciML/ModelingToolkit.jl#1867
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.
Could you add a test with ODEProblem?
No description provided.