Skip to content

Electrical switch added #387

New issue

Have a question about this project? No Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “No 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? No Sign in to your account

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Conversation

matthew-kapp
Copy link
Contributor

Checklist

  • Appropriate tests were added
  • Any code changes were done in a way that does not break public API
  • All documentation related to code changes were updated
  • The new code follows the
    contributor guidelines, in particular the SciML Style Guide and
    COLPRAC.
  • Any new documentation only uses public API

Additional context

  • It seems docs building is failing due to unrelated issue.

Copy link
Contributor

@baggepinnen baggepinnen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This component is placed in ideal_components.jl, but it's not an ideal switch? The docstring does not really indicate how the switch works, it appears to transition from conducting to not conducting exponentially rather that instantaneously like one would expect from a simple switch model?

@matthew-kapp
Copy link
Contributor Author

Yes, I should explain better in the docstring how it works.

Should this rather be modeled as a continuous event?

I wanted to avoid using events, but this might not be scalable (using delays).

@baggepinnen
Copy link
Contributor

I believe the modelica ideal switch model uses an event
https://github.com/modelica/ModelicaStandardLibrary/blob/99dff88bddedbe8b60bf81e778f7fe271e6035d9/Modelica/Electrical/Analog/Interfaces/IdealSwitch.mo#L15
(if statements like this translate to events in modelica)

It could make sense to include both an ideal and a filtered switch, but exactly how the filter works would be important, the default time constant of 1e-3 is an eternity for many analog electronics applications. It's likely also the case that the logic that provides the boolean input will introduce the event anyways, even with the filter you'd have a discontinuity in the first derivative, and many solvers would thus be unhappy without an event.

@matthew-kapp
Copy link
Contributor Author

Thanks @baggepinnen I would need to better understand events in MTK.

Good point - the step function in this case creates an event (as far as I understand).

@baggepinnen
Copy link
Contributor

I would need to better understand events in MTK.

MTK has the ability to automatically create the required events for functions that are known to be discontinuous. It's implemented, but not yet documented, you can see how it is invoked here
https://github.com/SciML/ModelingToolkit.jl/blob/30bf3723b2c7aefc69a153bd968ac9646174fadd/test/if_lifting.jl

structural_simplify(sys, additional_passes = [IfLifting])

In any case, I don't think you have to worry about creating the event in the switch component, the input is already assumed to be boolean, so it's the responsibility of the one creating the boolean in the first place to generate the event.

@matthew-kapp
Copy link
Contributor Author

This is very cool. I will look into this. So in theory, @continuous_events is no longer required then?

I agree the switch itself is not 'eventful', but I want to make sure that the test works with no delay/filtering, with the step (in my test case) being eventful, and it all being handled.

@matthew-kapp
Copy link
Contributor Author

@baggepinnen

  1. I have updated the docstring.
  2. I have removed the delay in the switch model.
  3. I have moved the delay from the switch to the step in the test.

Unfortunately, it seems IfLifting does not work for DAE's yet (An RC circuit is a DAE) - point 3 is therefore a short-term 'hack'.

Any further feedback would be much appreciated.

@ChrisRackauckas
Copy link
Member

Unfortunately, it seems IfLifting does not work for DAE's yet (An RC circuit is a DAE) - point 3 is therefore a short-term 'hack'.

I'm confused by two things here, (1) it is tested with DAEs? and (2) the RC circuit is an ODE?

@matthew-kapp
Copy link
Contributor Author

matthew-kapp commented Apr 29, 2025

Unfortunately, it seems IfLifting does not work for DAE's yet (An RC circuit is a DAE) - point 3 is therefore a short-term 'hack'.

I'm confused by two things here, (1) it is tested with DAEs? and (2) the RC circuit is an ODE?

I am not sure what the root cause is. Maybe I am doing something wrong. Let me copy the code I attempted and the outputs. The only difference with the test in the PR is that step(smooth=false) to create an event.

@mtkmodel SwitchTest begin
    @parameters begin
        V = 10.0
        C = 1.0
        R = 1.0
    end

    @components begin
        step = Step(start_time = 10.0, height = true, smooth = false)
        switch = Switch()
        voltage = Voltage()
        resistor = Resistor(R = R)
        capacitor = Capacitor(C = C, v = 0.0)
        ground = Ground()
    end

    @equations begin
        connect(voltage.p, switch.p)
        connect(switch.n, resistor.p)
        connect(resistor.n, capacitor.p)
        connect(voltage.n, capacitor.n, ground.g)
        connect(step.output, switch.input)
        voltage.V.u ~ V
    end
end

@named sys_ = SwitchTest()
sys = structural_simplify(sys_)
prob = ODEProblem(sys, [], (0.0, 25.0))

ODEProblem with uType Vector{Float64} and tType Float64. In-place: true
Initialization status: FULLY_DETERMINED
Non-trivial mass matrix: true
timespan: (0.0, 25.0)
u0: 2-element Vector{Float64}:
0.0
0.0

sol = solve(prob, Rodas4());

┌ Warning: At t=9.999999999999996, dt was forced below floating point epsilon 1.7763568394002505e-15, and step error estimate = 709.7217741210751. Aborting. There is either an error in your model specification or the true solution is unstable (or the true solution can not be represented in the precision of Float64).
└ @ SciMLBase C:\Users\matth.julia\packages\SciMLBase\R3Bkt\src\integrator_interface.jl:623

prob = ODEProblem(sys, [], (0.0, 25.0))

ODEProblem with uType Vector{Float64} and tType Float64. In-place: true
Initialization status: FULLY_DETERMINED
Non-trivial mass matrix: true
timespan: (0.0, 25.0)
u0: 2-element Vector{Float64}:
0.0
0.0

sol = solve(prob, Rodas4());

{
"name": "LoadError",
"message": "DAE initialization failed: your u0 did not satisfy the initialization requirements, normresid = 707106.7811865475 > abstol = 1.0e-6.\nIf you wish for the system to automatically change the algebraic variables to satisfy the algebraic constraints, please pass initializealg = BrownBasicInit() to solve (this option will require using OrdinaryDiffEqNonlinearSolve). If you wish to perform an initialization on the complete u0, please pass initializealg = ShampineCollocationInit() to solve. Note that initialization can be a very difficult process for DAEs and in many cases can be numerically intractable without symbolic manipulation of the system. For an automated system that will generate numerically stable initializations, see ModelingToolkit.jl structural simplification for more details.\n",

@matthew-kapp
Copy link
Contributor Author

The tests didn't start when I pushed an update?

The test passed on my machine, but not on here, so I was trying to figure out why.

@matthew-kapp
Copy link
Contributor Author

@baggepinnen @ChrisRackauckas I believe this PR is ready for review:

  1. Rebased with the docs fix
  2. Related tests pass

No Sign up for free to join this conversation on GitHub. Already have an account? No Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants