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

RC circuit demonstration doesn't work #70

Closed
dodoplus opened this issue Jun 9, 2022 · 9 comments
Closed

RC circuit demonstration doesn't work #70

dodoplus opened this issue Jun 9, 2022 · 9 comments

Comments

@dodoplus
Copy link

dodoplus commented Jun 9, 2022

The example uses ConstantVoltage which seems to be missing from latest release of the library.

Example is here: http://mtkstdlib.sciml.ai/dev/tutorials/rc_circuit/ as well as on the home page.

@kakila
Copy link

kakila commented Jun 9, 2022

The sources have changed, you need to create the input voltage using Blocks

 using ModelingToolkit, DifferentialEquations, Plots
 using ModelingToolkitStandardLibrary.Electrical
 using ModelingToolkitStandardLibrary.Blocks: Constant

resistance = 1.0
capacitance = 1.0
voltage = 1.0
@variables t
@named R = Resistor(R=resistance)
@named C = Capacitor(C=capacitance)
@named V = Voltage()
@named ground = Ground()
@named source = Constant(k=voltage)

rc_eqs = [
connect(V.p, R.p)
connect(R.n, C.p)
connect(C.n, V.n, ground.g)
connect(V.V, source.output)
]
@named rc_model = ODESystem(rc_eqs, t, systems=[R, C, V, source, ground])

sys = structural_simplify(rc_model)

prob = ODEProblem(sys, Pair[], (0, 10.0), [R.R => _R], jac = true)
sol = solve(prob)

p = plot(sol, vars = [C.v,R.i],
	title = "RC Circuit Demonstration",
	labels = ["Capacitor Voltage" "Resistor Current"])

@ChrisRackauckas
Copy link
Member

@ValentinKaisermayer can you enable doctests? That would've caught this.

@ValentinKaisermayer
Copy link
Contributor

I will do that. Should we add the electrical sources as explicit components as well?

@ChrisRackauckas
Copy link
Member

What do you mean?

@ValentinKaisermayer
Copy link
Contributor

The last PR removed a bunch of sources from Electrical (hence it was breaking), should we add those back in?

@ChrisRackauckas
Copy link
Member

yeah that should've been a breaking release. I know this is all a little loose since the library is still in its infant days, but we should start be clean up this repo a bit and make it properly deprecate deletions.

@ValentinKaisermayer
Copy link
Contributor

so should we add the sources back in?

@ChrisRackauckas
Copy link
Member

We might be fine this time, but this is the turning point. You did something breaking and two people instantly noticed it. If two people noticed it in hours, that means that the library has quite a bit of uptake by now. That's actually a bit surprising to me (I thought it was still in the "early pre-adoption phase"), but this makes it clear that it's adopted so it needs to start following the standard practices associated with a library people are have put into their dependency tree.

And on this note, I should probably prepare a release announcement 😅

@ValentinKaisermayer
Copy link
Contributor

Can be closed

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

No branches or pull requests

4 participants