MCTS.jl
MCTS.jl copied to clipboard
JET-suggested improvements and benchmarking with PkgBenchmarks
JET suggested improvements and benchmarking with PkgBenchmarks
This PR is a work in progress, and I am opening it for posterity since I have to move on for now and possibly to get other opinions on how to do this better.
I have tried using JET on MCTS.jl; this PR addresses issues highlighted by JET. I tried to benchmark the individual changes to see whether these issues were real.
Comparison of consecutive changes to baseline
The baseline should correspond to the latest master, with the addition of the benchmarking scripts.
Below are the results; the ratios are all to the same baseline, and all changes are additive, i.e., 30242a78 includes all the changes above it in the table.
# | hash | time ratio | memory ratio | comment |
---|---|---|---|---|
1 | f6a815db | 1.00 | 1.00 | Baseline. |
2 | 98450a1c | 1.15 | 1.00 | removed mutable where not needed, exposed sizehint externally. |
3 | b10aa869 | 1.21 | 0.98 | Add type to array creation in insert_node. |
4 | 30c18da2 | 1.08 | 0.98 | Replace 'nothing' with empty tree - remove uniontype in planner. |
5 | 76d103ee | 1.03 | 0.41 | Removed further abstract types from struct definitions. |
6 | 9c421b29 | 1.01 | 0.41 | Immutable SolvedRolloutEstimator. |
7 | 30242a78 | 1.02 | 0.41 | Removed abstract type from MCTSSolver struct. |
The time regressions are minor; some of them could be noise. The memory improvement is quite significant, but I haven't investigated which part of the commit is responsible yet.
Todos and benchmarking
I fumbled a bit with the benchmarking. First, I tried using BenchmarkTools.jl manually, then I tried using AirspeedVelocity.jl until finally settling on using PkgBenchmark.jl which eventually worked out with following workflow:
- edit/create benchmarks in
/benchmark/benchmarks.jl
and commit them to a benchmarking branch - git rebase the branch with commits I want to evaluate on top of the changes to the benchmark
-
] dev MCTS
in the benchmarking environment, use PkgBenchmark.jl to evaluate the commits.
What's still missing:
- I haven't checked what's causing the improvement in the memory
- the benchmarking script is not part of the repo; it is only attached below
- the initial commits contain code that the current master replaces; I should clean that up
- JET suggested changes to RolloutSimulator in POMDPs.jl are not part of this benchmark
- possibly, PkgBenchmarks could be included as part of the CI
Codecov Report
Attention: Patch coverage is 76.47059%
with 4 lines
in your changes are missing coverage. Please review.
Project coverage is 86.58%. Comparing base (
e6a4944
) to head (30242a7
).
Files | Patch % | Lines |
---|---|---|
src/vanilla.jl | 76.47% | 4 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## master #108 +/- ##
==========================================
- Coverage 86.74% 86.58% -0.17%
==========================================
Files 11 11
Lines 483 492 +9
==========================================
+ Hits 419 426 +7
- Misses 64 66 +2
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.