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

Proposed FAq doc updates #874

Merged
merged 5 commits into from
Jun 5, 2024
Merged

Proposed FAq doc updates #874

merged 5 commits into from
Jun 5, 2024

Conversation

TorkelE
Copy link
Member

@TorkelE TorkelE commented May 23, 2024

reuploaded due to a rebase issue

docs/src/faqs.md Outdated
@@ -28,33 +26,35 @@ observed(osys)
Let's solve the system and see how to index the solution using our symbolic
variables
```@example faq1
u0 = [rn.A => 1.0, rn.B => 2.0, rn.C => 0.0]
ps = [rn.k₊ => 1.0, rn.k₋ => 1.0]
Copy link
Member Author

Choose a reason for hiding this comment

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

Without this rewriting, running thsi example takes crazy (infinite? I have a feeling that we have completed docs with it recently, so might finish, never bothered waiting that long in the retests though, but I have waited a good bit) long times, see this issue: #886

Copy link
Member

Choose a reason for hiding this comment

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

Instead of changing our docs can you make an MTK level example and open issue there so this can get fixed?

Copy link
Member

@isaacsas isaacsas May 29, 2024

Choose a reason for hiding this comment

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

I assume the same issue occurs with ODESystems and is related to the new symbolic indexing stuff?

Copy link
Member Author

Choose a reason for hiding this comment

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

So right now I am trying to figure out what is going on enough that I can actually make an MTK level issue. Given that I have over 50 unfixed MTK issues right now, I don't feel very confident that this will be fixed anytime soon, unfortunately. It was not my intention to simply change the docs so that the issue can be ignored (hence I raised the Catalyst issue of a starter).

Copy link
Member Author

Choose a reason for hiding this comment

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

(The change is only proposed, showing that some type of update is required here and what that could be, it is not meant as a final suggestion how the doc should be)

Copy link
Member

Choose a reason for hiding this comment

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

Did this issue ever get resolved?

Copy link
Member Author

Choose a reason for hiding this comment

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

not yet

Copy link
Member

Choose a reason for hiding this comment

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

OK, let's go with this version of the mappings then.

@TorkelE TorkelE closed this Jun 1, 2024
@TorkelE TorkelE reopened this Jun 1, 2024
@TorkelE TorkelE closed this Jun 1, 2024
@TorkelE TorkelE reopened this Jun 1, 2024
@TorkelE TorkelE closed this Jun 1, 2024
@TorkelE TorkelE reopened this Jun 1, 2024
@TorkelE TorkelE closed this Jun 1, 2024
@TorkelE TorkelE reopened this Jun 1, 2024
docs/src/faqs.md Outdated
Comment on lines 37 to 59
```@example faq1
```julia
sol[osys.C]
```
or
```@example faq1
```julia
@unpack C = osys
sol[C]
```
To evaluate `C` at specific times and plot it we can just do
```@example faq1
```julia
t = range(0.0, 10.0, length=101)
plot(t, sol(t, idxs = C), label = "C(t)", xlabel = "t")
```
If we want to get multiple variables we can just do
```@example faq1
```julia
@unpack A, B = osys
sol(t, idxs = [A, B])
```
Plotting multiple variables using the SciML plot recipe can be achieved
like
```@example faq1
```julia
plot(sol; idxs = [A, B])
```
Copy link
Member

Choose a reason for hiding this comment

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

Why are these no longer dynamic?

Copy link
Member Author

Choose a reason for hiding this comment

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

I was trying to reduce runtime, sorry it got confused.

If you are happy with the change above I can reenable these, but if not we will have to figure something else out.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, if you reenable these I am fine to merge. I'd prefer we keep everything dynamic if we can.

@TorkelE TorkelE merged commit ae6cf0a into master Jun 5, 2024
7 checks passed
@TorkelE TorkelE deleted the proposed_faw_remake branch June 5, 2024 01:10
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.

2 participants