-
-
Notifications
You must be signed in to change notification settings - Fork 122
Add new tutorial: resonant circuit tutorial #480
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
Add new tutorial: resonant circuit tutorial #480
Conversation
Meine Änderungen in Solver_I.m
|
…ls into add-lc-circuit
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.
Looks good to me. @NiklasVin please also remove the case https://github.com/precice/matlab-bindings/tree/develop/tutorial from the matlab-bindings repo now that we moved it here.
I think it would be nice to get a review from somebody else, if possible. The installation process of the MATLAB bindings is not the best and adding the bindings to the path is not so easy, but let's try to solve this in the repository of the bindings. Not here.
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.
Structure-wise, this looks ready.
I have not tried to run it, but I would be interested, since we don't currently have any Matlab-based tutorial.
If everything is camera-ready today, we could squeeze it into this release, if you feel confident about it. But I would not wait for it, because we are already quite late.
Moved to tutorials repo via precice/tutorials#480.
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.
I found another minor simplification. @NiklasVin if you think this is reasonable, please commit these suggestions to the branch.
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.
I think we are converging here! Can you please go once more through all the comments, briefly write an answer (if necessary) and resolve the conversations. @MakisH Do you want to do another quick check, approve, and merge (I cannot approve, because I created this PR originally).
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.
Nice! 🚀
I still have not tested it, but that would take much longer, as I first have to finish other tasks. Feel free to merge after fixing a few minor issues in the README.
See precice/matlab-bindings#29