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 recursive structure and setup precompilation #2301

Merged
merged 1 commit into from
Oct 6, 2023

Conversation

ChrisRackauckas
Copy link
Member

The recursive structure of SystemStructure was giving precompilation an issue so I removed that module. Then I setup precompilation on the basic interface. Test case:

@time using ModelingToolkit
@time using DifferentialEquations
@time using Plots
@time begin
    @variables t x(t)
    D = Differential(t)
    @named sys = ODESystem([D(x) ~ -x])
end;
@time prob = ODEProblem(structural_simplify(sys), [x => 30.0], (0, 100), [], jac = true);
@time sol = solve(prob);
@time plot(sol, idxs=[x]);

Before:

11.082586 seconds (19.32 M allocations: 1.098 GiB, 4.09% gc time, 0.71% compilation time: 87% of which was recompilation)
0.639738 seconds (661.39 k allocations: 101.321 MiB, 4.33% gc time, 6.46% compilation time)
3.703724 seconds (5.71 M allocations: 322.840 MiB, 5.22% gc time, 9.92% compilation time: 86% of which was recompilation)
7.795297 seconds (8.25 M allocations: 483.041 MiB, 2.50% gc time, 99.88% compilation time)
21.719376 seconds (44.11 M allocations: 2.485 GiB, 5.68% gc time, 99.48% compilation time)
2.602250 seconds (4.04 M allocations: 253.058 MiB, 4.60% gc time, 99.90% compilation time)
2.450509 seconds (5.17 M allocations: 332.101 MiB, 5.89% gc time, 99.41% compilation time: 30% of which was recompilation)

After:

9.129141 seconds (22.77 M allocations: 1.291 GiB, 4.65% gc time, 0.62% compilation time: 87% of which was recompilation)
0.784464 seconds (667.59 k allocations: 101.524 MiB, 3.95% gc time, 4.16% compilation time)
3.111142 seconds (5.42 M allocations: 305.594 MiB, 3.82% gc time, 6.39% compilation time: 82% of which was recompilation)
0.105567 seconds (157.39 k allocations: 10.522 MiB, 8.81% gc time, 95.49% compilation time: 74% of which was recompilation)
1.993642 seconds (4.03 M allocations: 218.310 MiB, 2.69% gc time, 96.95% compilation time: 82% of which was recompilation)
1.806758 seconds (4.06 M allocations: 254.371 MiB, 4.44% gc time, 99.91% compilation time)
1.694666 seconds (5.27 M allocations: 339.088 MiB, 6.18% gc time, 99.39% compilation time: 31% of which was recompilation)

And that's on v1.9, so v1.10 should be even better. 20 seconds off hehe.

@@ -156,7 +158,6 @@ include("systems/dependency_graphs.jl")
include("clock.jl")
include("discretedomain.jl")
include("systems/systemstructure.jl")
using .SystemStructures
Copy link
Member

Choose a reason for hiding this comment

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

Is this deliberate? It would be slightly breaking.

Copy link
Member Author

Choose a reason for hiding this comment

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

It is deliberate. How is it breaking?

Copy link
Member

@YingboMa YingboMa Oct 6, 2023

Choose a reason for hiding this comment

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

It will break ModelingToolkit.X as a shorthand for ModelingToolkit.SystemStructures.X. Can we use it at least after precompilation?

Copy link
Member Author

Choose a reason for hiding this comment

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

SystemStructures is no longer a module.

Copy link
Member

@YingboMa YingboMa Oct 6, 2023

Choose a reason for hiding this comment

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

Ah, well, it's breaking when people actually do ModelingToolkit.SystemStructures.X then 😅.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's not public... right?

@YingboMa YingboMa force-pushed the precompile_workload branch from 28ba51a to 61ac8b5 Compare October 6, 2023 13:30
@ChrisRackauckas
Copy link
Member Author

v1.10 is nice:

  3.200767 seconds (4.81 M allocations: 301.123 MiB, 2.37% gc time, 0.55% compilation time)
  0.165489 seconds (168.82 k allocations: 10.458 MiB, 17.35% compilation time)
  1.053160 seconds (1.76 M allocations: 98.635 MiB, 3.62% gc time, 15.93% compilation time: 80% of which was recompilation)
  0.317384 seconds (901.99 k allocations: 59.005 MiB, 99.53% compilation time: 88% of which was recompilation)
  1.851974 seconds (3.11 M allocations: 208.672 MiB, 1.25% gc time, 99.41% compilation time: 90% of which was recompilation)
  1.969298 seconds (3.64 M allocations: 244.764 MiB, 0.66% gc time, 99.97% compilation time)
  1.640871 seconds (4.28 M allocations: 289.529 MiB, 2.00% gc time, 99.69% compilation time: 18% of which was recompilation)

test/clock.jl Outdated Show resolved Hide resolved
The recursive structure of SystemStructure was giving precompilation an issue so I removed that module. Then I setup precompilation on the basic interface.  Test case:

```julia
@time using ModelingToolkit
@time using DifferentialEquations
@time using Plots
@time begin
    @variables t x(t)
    D = Differential(t)
    @nAmed sys = ODESystem([D(x) ~ -x])
end;
@time prob = ODEProblem(structural_simplify(sys), [x => 30.0], (0, 100), [], jac = true);
@time sol = solve(prob);
@time plot(sol, idxs=[x]);
```

Before:

```
11.082586 seconds (19.32 M allocations: 1.098 GiB, 4.09% gc time, 0.71% compilation time: 87% of which was recompilation)
0.639738 seconds (661.39 k allocations: 101.321 MiB, 4.33% gc time, 6.46% compilation time)
3.703724 seconds (5.71 M allocations: 322.840 MiB, 5.22% gc time, 9.92% compilation time: 86% of which was recompilation)
7.795297 seconds (8.25 M allocations: 483.041 MiB, 2.50% gc time, 99.88% compilation time)
21.719376 seconds (44.11 M allocations: 2.485 GiB, 5.68% gc time, 99.48% compilation time)
2.602250 seconds (4.04 M allocations: 253.058 MiB, 4.60% gc time, 99.90% compilation time)
2.450509 seconds (5.17 M allocations: 332.101 MiB, 5.89% gc time, 99.41% compilation time: 30% of which was recompilation)
```

After:

```
9.129141 seconds (22.77 M allocations: 1.291 GiB, 4.65% gc time, 0.62% compilation time: 87% of which was recompilation)
0.784464 seconds (667.59 k allocations: 101.524 MiB, 3.95% gc time, 4.16% compilation time)
3.111142 seconds (5.42 M allocations: 305.594 MiB, 3.82% gc time, 6.39% compilation time: 82% of which was recompilation)
0.105567 seconds (157.39 k allocations: 10.522 MiB, 8.81% gc time, 95.49% compilation time: 74% of which was recompilation)
1.993642 seconds (4.03 M allocations: 218.310 MiB, 2.69% gc time, 96.95% compilation time: 82% of which was recompilation)
1.806758 seconds (4.06 M allocations: 254.371 MiB, 4.44% gc time, 99.91% compilation time)
1.694666 seconds (5.27 M allocations: 339.088 MiB, 6.18% gc time, 99.39% compilation time: 31% of which was recompilation)
```

And that's on v1.9, so v1.10 should be even better. 20 seconds off hehe.
@YingboMa YingboMa force-pushed the precompile_workload branch from 2d819fa to 6e795b8 Compare October 6, 2023 16:03
@ChrisRackauckas ChrisRackauckas merged commit 7949761 into master Oct 6, 2023
25 of 33 checks passed
@ChrisRackauckas ChrisRackauckas deleted the precompile_workload branch October 6, 2023 20:13
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