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

Fix metadata prop #624

Merged
merged 2 commits into from
Jul 17, 2024
Merged

Fix metadata prop #624

merged 2 commits into from
Jul 17, 2024

Conversation

YingboMa
Copy link
Member

@YingboMa YingboMa commented Jul 17, 2024

-(-x) will simplify back to x and setting metadata on the simplified result is illegal.

Fixes SciML/ModelingToolkit.jl#2855

Copy link
Contributor

github-actions bot commented Jul 17, 2024

Benchmark Results

master e16109b... master/e16109b33608a7...
overhead/acrule/a+2 0.731 ± 0.019 μs 0.764 ± 0.022 μs 0.958
overhead/acrule/a+2+b 0.709 ± 0.02 μs 0.737 ± 0.019 μs 0.961
overhead/acrule/a+b 0.258 ± 0.0098 μs 0.265 ± 0.0088 μs 0.977
overhead/acrule/noop:Int 25.4 ± 0.93 ns 25.5 ± 0.93 ns 0.993
overhead/acrule/noop:Sym 0.0367 ± 0.0057 μs 0.0364 ± 0.0049 μs 1.01
overhead/rule/noop:Int 0.0439 ± 0.00078 μs 0.0447 ± 0.0013 μs 0.982
overhead/rule/noop:Sym 0.0562 ± 0.003 μs 0.0562 ± 0.0031 μs 0.999
overhead/rule/noop:Term 0.0563 ± 0.0027 μs 0.0589 ± 0.0032 μs 0.956
overhead/ruleset/noop:Int 0.132 ± 0.0026 μs 0.133 ± 0.0029 μs 0.991
overhead/ruleset/noop:Sym 0.158 ± 0.0061 μs 0.163 ± 0.0068 μs 0.97
overhead/ruleset/noop:Term 3.72 ± 0.14 μs 3.61 ± 0.14 μs 1.03
overhead/simplify/noop:Int 0.156 ± 0.033 μs 0.152 ± 0.0034 μs 1.03
overhead/simplify/noop:Sym 0.154 ± 0.0034 μs 0.165 ± 0.0026 μs 0.932
overhead/simplify/noop:Term 0.0381 ± 0.0021 ms 0.0371 ± 0.0016 ms 1.03
overhead/simplify/randterm (+, *):serial 0.0906 ± 0.0012 s 0.0898 ± 0.0015 s 1.01
overhead/simplify/randterm (+, *):thread 0.0533 ± 0.033 s 0.0522 ± 0.033 s 1.02
overhead/simplify/randterm (/, *):serial 0.227 ± 0.0078 ms 0.216 ± 0.0062 ms 1.05
overhead/simplify/randterm (/, *):thread 0.262 ± 0.0083 ms 0.254 ± 0.008 ms 1.03
overhead/substitute/a 0.0582 ± 0.0014 ms 0.0553 ± 0.0016 ms 1.05
overhead/substitute/a,b 0.0519 ± 0.0015 ms 0.0505 ± 0.0016 ms 1.03
overhead/substitute/a,b,c 18.6 ± 0.73 μs 18.1 ± 0.77 μs 1.03
polyform/easy_iszero 0.0342 ± 0.0017 ms 0.0321 ± 0.0017 ms 1.07
polyform/isone 2.79 ± 0.009 ns 3.1 ± 0.01 ns 0.903
polyform/iszero 1.26 ± 0.042 ms 1.19 ± 0.032 ms 1.06
polyform/simplify_fractions 1.87 ± 0.059 ms 1.83 ± 0.049 ms 1.02
time_to_load 4.69 ± 0.035 s 4.7 ± 0.029 s 0.999

Benchmark Plots

A plot of the benchmark results have been uploaded as an artifact to the workflow run for this PR.
Go to "Actions"->"Benchmark a pull request"->[the most recent run]->"Artifacts" (at the bottom).

`-(-x)` will simplify back to `x` and setting metadata on the simplified result
is illegal.
@YingboMa
Copy link
Member Author

Tests failures seem unrelated.

@ChrisRackauckas
Copy link
Member

Is SciML/ModelingToolkit.jl#2855 related?

@YingboMa
Copy link
Member Author

It should if fix it as well.

julia> buggy(-1) # [-B₊x(t) ~ 0], as expected
1-element Vector{Equation}:
 -B₊x(t) ~ 0

julia> buggy(0)  # [0 ~ 0], as expected
1-element Vector{Equation}:
 0 ~ 0

julia> buggy(1)  # [x(t) ~ 0], but expected [B₊x(t) ~ 0]
1-element Vector{Equation}:
 B₊x(t) ~ 0

julia> buggy(2)  # [2B₊x(t) ~ 0], as expected
1-element Vector{Equation}:
 2B₊x(t) ~ 0

@ChrisRackauckas
Copy link
Member

Well that explains that. Perfect.

@ChrisRackauckas ChrisRackauckas merged commit d2a0b35 into master Jul 17, 2024
9 of 12 checks passed
@ChrisRackauckas ChrisRackauckas deleted the myb/fix_md branch July 17, 2024 21:51
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.

Subsystem not namespaced when substituted coefficient is 1
2 participants