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

Anandpathak31/issue5 #6

Merged
merged 20 commits into from
Sep 27, 2024
Merged

Anandpathak31/issue5 #6

merged 20 commits into from
Sep 27, 2024

Conversation

anandpathak31
Copy link
Contributor

No description provided.

Copy link
Contributor

@MasonProtter MasonProtter left a comment

Choose a reason for hiding this comment

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

Here's some comments I have on the beginner tutorial, feel free to take or leave any of them, they're just suggestions.

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

## Plotting graphs

We can plot graphs in Julia using the `Plots.jl` package. To add this package we use the terminal:
Copy link
Contributor

Choose a reason for hiding this comment

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

we should consider switching this to Makie

Comment on lines +93 to +94
# Define the time variable
@variables t
Copy link
Contributor

Choose a reason for hiding this comment

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

It's okay to do this, but it might be a better idea to just use MTK's builtin t and D, i.e.

using ModelingToolkit: t_nounits as t, D_nounits as D

since that's the more recommended way of doing things nowadays, you'll have less clashing definitions of t and D floating around, but it's also not a big deal


```@example beginner_tutorial
# Define the differential operator
D = Differential(t)
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment as above

docs/src/tutorials/beginner_tutorial.md Outdated Show resolved Hide resolved
Comment on lines +176 to +178
@variables t v(t) u(t)
@parameters a b c d I
D = Differential(t)
Copy link
Contributor

Choose a reason for hiding this comment

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

consider switching to using ModelingToolkit: t_nounits as t, D_nounits as D

Copy link
Contributor

@helmutstrey helmutstrey left a 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.

@anandpathak31
Copy link
Contributor Author

@harisorgn this PR is complete. I am not merging to avoid overwriting the master which might be ahead in commits.
Please review.

@harisorgn harisorgn merged commit 47529a2 into main Sep 27, 2024
1 check failed
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.

5 participants