Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

fix: array parameters in nested @mtkmodel components#3944

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
baggepinnen wants to merge2 commits intomaster
base:master
Choose a base branch
Loading
fromfb/defaultarraybug

Conversation

@baggepinnen
Copy link
Contributor

@baggepinnenbaggepinnen commentedSep 25, 2025
edited
Loading

attempt toclose#2813

This only handles defaults that are a single Num, liker=r_def, it does not yet handler_def being an expression with multiple variables

@github-actions
Copy link
Contributor

github-actionsbot commentedSep 25, 2025
edited
Loading

Benchmark Results (Julia vlts)

Time benchmarks
master42688d5...master /42688d5...
ODEProblem0.0858 ± 0.0071 s0.0833 ± 0.0068 s1.03 ± 0.12
init0.0598 ± 0.0024 ms0.0587 ± 0.0016 ms1.02 ± 0.05
large_parameter_init/ODEProblem1.11 ± 0.0016 s1.11 ± 0.0037 s0.999 ± 0.0037
large_parameter_init/init0.111 ± 0.0044 ms0.109 ± 0.0028 ms1.01 ± 0.048
mtkcompile0.0367 ± 0.0033 s0.0378 ± 0.0016 s0.971 ± 0.096
time_to_load4.99 ± 0.026 s5.08 ± 0.018 s0.982 ± 0.0062
Memory benchmarks
master42688d5...master /42688d5...
ODEProblem0.587 M allocs: 19.9 MB0.587 M allocs: 19.9 MB1
init0.892 k allocs: 0.0463 MB0.892 k allocs: 0.0463 MB1
large_parameter_init/ODEProblem4.32 M allocs: 0.168 GB4.3 M allocs: 0.166 GB1.01
large_parameter_init/init1.74 k allocs: 0.0858 MB1.74 k allocs: 0.0858 MB1
mtkcompile0.237 M allocs: 8.75 MB0.243 M allocs: 9.02 MB0.971
time_to_load0.153 k allocs: 14.5 kB0.153 k allocs: 14.5 kB1

@github-actions
Copy link
Contributor

github-actionsbot commentedSep 25, 2025
edited
Loading

Benchmark Results (Julia v1)

Time benchmarks
master42688d5...master /42688d5...
ODEProblem0.0793 ± 0.0041 s0.0765 ± 0.0055 s1.04 ± 0.092
init0.0526 ± 0.012 ms0.0524 ± 0.011 ms1 ± 0.31
large_parameter_init/ODEProblem1.09 ± 0.015 s1.06 ± 0.021 s1.03 ± 0.024
large_parameter_init/init0.094 ± 0.0091 ms0.0939 ± 0.01 ms1 ± 0.15
mtkcompile0.0344 ± 0.00098 s0.0343 ± 0.00062 s1 ± 0.034
time_to_load4.92 ± 0.061 s4.9 ± 0.044 s1.01 ± 0.015
Memory benchmarks
master42688d5...master /42688d5...
ODEProblem0.577 M allocs: 18.8 MB0.578 M allocs: 18.8 MB1
init0.799 k allocs: 30.9 kB0.799 k allocs: 30.9 kB1
large_parameter_init/ODEProblem4.39 M allocs: 0.16 GB4.4 M allocs: 0.159 GB1
large_parameter_init/init1.65 k allocs: 0.0696 MB1.65 k allocs: 0.0696 MB1
mtkcompile0.236 M allocs: 8.17 MB0.242 M allocs: 8.38 MB0.975
time_to_load0.159 k allocs: 11.2 kB0.159 k allocs: 11.2 kB1

for varin expr_vars
var_unwrapped=unwrap(var)
# If any variable in the expression is from parent scope, don't namespace
ifisparameter(var_unwrapped)&&!(var_unwrappedin sys_params|| var_unwrappedin sys_unknowns)
Copy link
Contributor

Choose a reason for hiding this comment

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

[JuliaFormatter]reported byreviewdog 🐶

Suggested change
ifisparameter(var_unwrapped)&&!(var_unwrappedin sys_params|| var_unwrappedin sys_unknowns)
ifisparameter(var_unwrapped)&&
!(var_unwrappedin sys_params|| var_unwrappedin sys_unknowns)

Comment on lines +1170 to +1171
Dict((isparameter(k)?parameters(sys, k):unknowns(sys, k))=>
(should_namespace(v)?namespace_expr(v, sys): v)
Copy link
Contributor

Choose a reason for hiding this comment

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

[JuliaFormatter]reported byreviewdog 🐶

Suggested change
Dict((isparameter(k)?parameters(sys, k):unknowns(sys, k))=>
(should_namespace(v)?namespace_expr(v, sys): v)
Dict((isparameter(k)?parameters(sys, k) :
unknowns(sys, k))=>(should_namespace(v)?namespace_expr(v, sys): v)

Copy link
Member

@AayushSabharwalAayushSabharwal left a comment

Choose a reason for hiding this comment

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

Tests fail

@baggepinnen
Copy link
ContributorAuthor

I don't think I have sufficient insight into the internals to make everything pass. This was enough to allow me to move forward on a time critical problem, so I'll probably leave it here for now

@AayushSabharwal
Copy link
Member

Makes sense, I'll try and take a look when I can.

@baggepinnen
Copy link
ContributorAuthor

Here's another test case that fails both on latest release and this PR branch, wrong number of defaults generated

using ModelingToolkitusing ModelingToolkit: t_nounits as t, D_nounits as D@mtkmodel inner2begin@parametersbegin        a[1:2]= aend@variablesbeginx(t)[1:2]=zeros(2)end@equationsbeginD(x)~ aendend@mtkmodel outer2begin@parametersbegin        a1=1        a2=1end@componentsbegin        i=inner2(a=[a1,a2])end@variablesbeginy(t)[1:2]=zeros(2)dy(t)[1:2]end@equationsbeginD(y)~ dy        dy~ [a1, a2]endend@named o=outer2()o=complete(o)ssys=mtkcompile(o)varmap= []prob=ODEProblem(ssys, varmap, (0,1))

This looks fishy

julia> MTK=ModelingToolkit;filter(kvp->!MTK.isinitial(kvp[1])&&!MTK.isparameter(kvp[1]),defaults(ssys))Dict{Any, Any} with6 entries:  (i₊x(t))[2]=>0.0  (i₊x(t))[1]=>0.0  (y(t))[2]=>0.0  (y(t))[1]=>0.0  (dy(t))[2]=>NoValue()  (dy(t))[1]=>NoValue()

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@AayushSabharwalAayushSabharwalAayushSabharwal left review comments

@github-actionsgithub-actions[bot]github-actions[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

parameter default given by other parameter leads to incorrect codegen

3 participants

@baggepinnen@AayushSabharwal

[8]ページ先頭

©2009-2025 Movatter.jp