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

WIP: make tests run faster #2277

Closed
wants to merge 7 commits into from
Closed

WIP: make tests run faster #2277

wants to merge 7 commits into from

Conversation

odow
Copy link
Member

@odow odow commented Jul 12, 2020

This PR is an attempt to make the JuMP tests run faster.

The first commit is a general tidying commit:

  • Removes unneeded imports
  • Remove DualNumbers in favor of ForwardDiff
  • Adds imports each test file so that it can be run independently of runtests.jl, or in an arbitrary order

The next commits refactor the worst-performing files to speed them up.

In every case, the overwhelming majority of the time is spent compiling methods instead of doing work.

For example, running test/operator.jl (slightly modified to call TestOperator.runtests() twice) on my machine yields:

oscar@Oscars-MBP JuMP % ~/julia1.3 --project=. --color=yes test/operator.jl 
 31.475031 seconds (89.82 M allocations: 4.505 GiB, 5.54% gc time)
  0.025482 seconds (64.32 k allocations: 5.131 MiB)

Yes, that is a 1200 fold difference in run time the second time round 😢.
Running it with -O0 drops the time further.

oscar@Oscars-MBP JuMP % ~/julia1.3 --project=. --color=yes -O0 test/operator.jl
 23.202877 seconds (89.82 M allocations: 4.505 GiB, 7.17% gc time)
  0.023707 seconds (64.97 k allocations: 5.151 MiB)

Current top-5 run times in seconds

commit ma operator file_formats constraint variable TOTAL(5)
start (s) 200 76 64 59 33 432
latest (s) 101 40 35 51 34 261
change -50% -47% -45% -13% +0% -40%

Latest master build of Travis:
image

Latest Travis build for this branch:
image

TODO before merging:

  • Consider whether to cherry pick commits 2 to N into separate PRs
  • Can we run travis with -O0?

@codecov
Copy link

codecov bot commented Jul 12, 2020

Codecov Report

Merging #2277 into master will increase coverage by 0.13%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2277      +/-   ##
==========================================
+ Coverage   91.13%   91.26%   +0.13%     
==========================================
  Files          42       42              
  Lines        4194     4234      +40     
==========================================
+ Hits         3822     3864      +42     
+ Misses        372      370       -2     
Impacted Files Coverage Δ
src/variables.jl 97.57% <100.00%> (+1.43%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 42668d1...4adb02b. Read the comment docs.

- Make it so individual test files can be run
- Time which tests take the most time
  - Currently, MutableArithmetics is the worst offender
- Remove unneeded test entries in Project.toml
odow added 6 commits July 12, 2020 20:08
Benchmarks suggest a 40% reduction in runtime, but this is still almost
all compile time. Runing Julia with -O0 gives another 30% reduction.
…tsets

In addition, remove some tests. We don't need to test every type of file
format, just the ability of JuMP to talk to MOI.FileFormats.

This avoids having to compile the CBF, LP, and MPS modules just to
read/write a single file.
…ted testsets

In addition, disable JuMPExtension tests. They double our testing time
without adding much in the way of extra coverage.
@mlubin
Copy link
Member

mlubin commented Jul 13, 2020

+1 for splitting this into smaller PRs

@odow
Copy link
Member Author

odow commented Jul 13, 2020

+1 for splitting this into smaller PRs

Agreed. I was just trying to see the cumulative impact. Which is okay, but not overwhelming.

@KristofferC
Copy link

Avoiding big testsets (like the ones enacpsulating the whole file as

@testset "Nonlinear" begin
does) is also something to try.

@IainNZ
Copy link
Collaborator

IainNZ commented Aug 2, 2020

Looks like Pukeko.jl needed :D

@odow
Copy link
Member Author

odow commented Aug 8, 2020

Closed by separate PRs.

@odow odow closed this Aug 8, 2020
@odow odow deleted the od/tests branch August 8, 2020 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants