8000 Minor doc updates by TorkelE · Pull Request #287 · sebapersson/PEtab.jl · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Minor doc updates #287

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
May 22, 2025
Merged

Minor doc updates #287

merged 4 commits into from
May 22, 2025

Conversation

TorkelE
Copy link
Contributor
@TorkelE TorkelE commented May 20, 2025

Should make DSL-based model declaration better.

@sebapersson
Copy link
Owner

From the Doc CI failing it seems D must still be defined in this example:

│ using Distributions, ModelingToolkit, OrdinaryDiffEq, Plots
│ @mtkmodel SYS begin
│     @parameters begin
│         b1
│         b2
│     end
│     @variables begin
│         x(t) = 0.0
│     end
│     @equations begin
│         D(x) ~ b2 * (b1 - x)
│     end
│ end
│ @mtkbuild sys = SYS()

Or maybe it is better to use Differential (as I believe it is called in MTK)?

@TorkelE
Copy link
Contributor Author
TorkelE commented May 20, 2025

You are right, the D must be defined.

Differential can be used, but must also specify that it is the time derivative:

@equations begin
    Differential(t)(x) ~ b2 * (b1 - x)
end

Do you have a preference? I think right now the @mtkmodel macro don't see enough use that there is that an established syntax for how to use. I imagine it might seem some improvements in the long term when the non-macro workflow has been fully finalised.

@sebapersson
Copy link
Owner

Do you have a preference?

I think I would prefer D as it is cleaner.

@TorkelE
Copy link
Contributor Author
TorkelE commented May 20, 2025

I have updated to

using ModelingToolkit: D_nounits as D

from

using ModelingToolkit: t_nounits as t, D_nounits as D

@sebapersson
Copy link
Owner

The Doc error is because ModelingToolkit ODESystem cannot infer the time variable. Test failures is not your fault, it is a recent CI bug that popped up (saw it first in #285 ). I believe this is a fix I have not had time to test yet: https://discourse.julialang.org/t/problem-with-github-action-julia-docdeploy/128789

@sebapersson
Copy link
Owner

I can fix the CI error later if you fix the doc-error.

@sebapersson
Copy link
Owner

I think the current error should just be due to PEtabEvent using t in the docs.

@TorkelE
Copy link
Contributor Author
TorkelE commented May 21, 2025

Ok, I will leav this one to you to finish of then?
(no hurry about timelines though)

@sebapersson
Copy link
Owner

Ok, I will leav this one to you to finish of then?

Yes, I will fix the last CI problems

@sebapersson sebapersson merged commit 1b0dd96 into sebapersson:main May 22, 2025
1 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@TorkelE @sebapersson
0