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

docs: add cauer low pass analog filter example #240

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

Conversation

jvaverka
Copy link

@jvaverka jvaverka commented Nov 8, 2023

No description provided.

Copy link

codecov bot commented Nov 8, 2023

Codecov Report

Merging #240 (1f2e0a5) into main (c85424e) will decrease coverage by 0.03%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #240      +/-   ##
==========================================
- Coverage   16.02%   15.99%   -0.03%     
==========================================
  Files          48       48              
  Lines        1629     1638       +9     
==========================================
+ Hits          261      262       +1     
- Misses       1368     1376       +8     

see 3 files with indirect coverage changes

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

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.

Nice, thanks for this detailed tutorial :)

Would it be useful to add units to the capacitance and inductance parameters?
@ven-k has been adding units to components in

docs/src/tutorials/cauer_low_pass_analog.md Outdated Show resolved Hide resolved
docs/src/tutorials/cauer_low_pass_analog.md Outdated Show resolved Hide resolved
docs/src/tutorials/cauer_low_pass_analog.md Outdated Show resolved Hide resolved

```plaintext
ERROR: Default algorithm choices require DifferentialEquations.jl.
Please specify an algorithm (e.g., `solve(prob, Tsit5())` or
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps we can suggest using DifferentialEquations and solve(prob) instead of OrdinaryDiffEq?
(While keeping the note+url on where they can find all the solvers they can specify if they want to.)

Copy link
Author

@jvaverka jvaverka Nov 9, 2023

Choose a reason for hiding this comment

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

You mean for all the tutorials? Seems like a good idea to me. My understanding of the trade-off being OrdinaryDiffEq.jl is more lightweight and therefore faster to load, but DifferentialEquations.jl is more robust and offers conveniences to new users - which is useful for readers going through the tutorials.

Should I leave the note, and open an issue to switch over all tutorials and update docs/Project.toml?

@ven-k
Copy link
Member

ven-k commented Nov 9, 2023

I'm keeping the Units PR open till upcoming breaking release; it would invalidate any model that use these along with unitless parameters. I can add units to this tutorial in that PR.

@jvaverka
Copy link
Author

jvaverka commented Nov 9, 2023

Nice, thanks for this detailed tutorial :)

Would it be useful to add units to the capacitance and inductance parameters? @ven-k has been adding units to components in

I would be happy to add units after #214 is merged. Originally, I wrote the example to include units but then hit errors and had to remove them.

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