-
-
Notifications
You must be signed in to change notification settings - Fork 100
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
base: develop
Are you sure you want to change the base?
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.
Co-authored-by: Gerasimos Chourdakis <chourdak@in.tum.de>
@@ -0,0 +1 @@ | |||
- Added new [resonant-circuit tutorial](https://precice.org/tutorials-resonant-circuit.html) with MATLAB (migrated from [matlab-bindings repository](https://github.com/precice/matlab-bindings and updated to follow tutorials structure)). |
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.
@MakisH the CI pipeline complains here. I think we need to ignore certain rules that we check for the README.md
files in these kind of files. Or should we just ignore the complete folder?
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 I broke that exception recently. Ignore it, and I will fix it in a separate PR.
See precice/matlab-bindings#29