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

Adapt IT1788 tests to use Base.Test and run them in parallel #235

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

lbenet
Copy link
Member

@lbenet lbenet commented Mar 5, 2017

Note that in ITF1788_tests.jl does not yet include running the tests in
ieee1788-constructors.jl; some of these tests do not pass yet, since
some functionality is lacking.

Note that in ITF1788_tests.jl does not yet include running the tests in
ieee1788-constructors.jl; some of these tests do not pass yet, since
some functionality is lacking.
@lbenet
Copy link
Member Author

lbenet commented Mar 5, 2017

This completes #205

@lbenet
Copy link
Member Author

lbenet commented Mar 5, 2017

Checking what went wrong...

@dpsanders
Copy link
Member

This is great work!

What does addprocs() with no arguments do?

@dpsanders
Copy link
Member

The failing tests seem to be since a file is missing.

@lbenet
Copy link
Member Author

lbenet commented Mar 5, 2017

Thanks!

It adds all possible workers of your machine (worker 1 is master), so you use them all to do the tests.

@lbenet
Copy link
Member Author

lbenet commented Mar 5, 2017

Some tests, not (yet) related to the ITF1788 suite are not passing in travis.

@lbenet
Copy link
Member Author

lbenet commented Mar 5, 2017

Sorry, it is indeed related to the ITF1788 suite....

@dpsanders
Copy link
Member

It looks like you forgot to commit the bool test file?

@lbenet
Copy link
Member Author

lbenet commented Mar 5, 2017

But I see it there. Maybe a question of path in travis...?

@dpsanders
Copy link
Member

Not sure.

@lbenet
Copy link
Member Author

lbenet commented Mar 5, 2017

Indeed, it is the path: It is looking for "/home/travis/.julia/v0.5/ValidatedNumerics/test/libieeep1788_tests_bool.jl" while it should be "/home/travis/.julia/v0.5/ValidatedNumerics/test/ITF1788_tests/libieeep1788_tests_bool.jl"

@dpsanders
Copy link
Member

Ah OK, so you can just add the directory name. Or. Maybe do a "cd".

It is maybe worth looking in the Julia base tests to see how they do it to be compatible with Windows at some point in the future. Or just leave it unixy for now for simplicity.

@dpsanders
Copy link
Member

Ah you can just use joinpath

@lbenet
Copy link
Member Author

lbenet commented Mar 5, 2017

I just pushed a new commit that should solve this; if you tell me how to use it, I do change it

@lbenet
Copy link
Member Author

lbenet commented Mar 5, 2017

Another try...

@lbenet
Copy link
Member Author

lbenet commented Mar 5, 2017

Tests are running.... finally!

@dpsanders
Copy link
Member

Great! Is it possible to parallelise the non itf tests (ours) too?

@lbenet
Copy link
Member Author

lbenet commented Mar 5, 2017

The scheme we have now is pretty naive: each worker includes a test file independently.

This can be extended for all included test files, but we must be sure about two things: the master (worker 1) has to be in charge of the very first test, since it has to precompile. The second thing is to have all workers do the ITF1788 tests, which take really long.

Shall I implement this in this PR?

@codecov-io
Copy link

codecov-io commented Mar 5, 2017

Codecov Report

Merging #235 into master will decrease coverage by -91.86%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           master   #235       +/-   ##
=========================================
- Coverage   91.85%     0%   -91.86%     
=========================================
  Files          26     26               
  Lines        1044   1021       -23     
=========================================
- Hits          959      0      -959     
- Misses         85   1021      +936
Impacted Files Coverage Δ
src/intervals/intervals.jl 0% <0%> (-100%)
src/ValidatedNumerics.jl 0% <0%> (-100%)
src/intervals/special.jl 0% <0%> (-100%)
src/multidim/setdiff.jl 0% <0%> (-100%)
src/intervals/hyperbolic.jl 0% <0%> (-100%)
src/root_finding/bisect.jl 0% <0%> (-100%)
src/intervals/trigonometric.jl 0% <0%> (-99.1%)
src/intervals/arithmetic.jl 0% <0%> (-98.65%)
src/parsing.jl 0% <0%> (-96.97%)
src/intervals/precision.jl 0% <0%> (-96.56%)
... and 14 more

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 a24a7f3...92b2b49. Read the comment docs.

@coveralls
Copy link

Coverage Status

Coverage decreased (-12.2%) to 79.693% when pulling 8a8a45e on lb/itf1788_Base.Test into a24a7f3 on master.

@dpsanders
Copy link
Member

I suggest you make a new pr in a new branch based in this branch. In that way we can easily see. If it has any effect on the total time to run the tests

@lbenet
Copy link
Member Author

lbenet commented Mar 5, 2017

Travis is giving problems: while tests pass in 0.4, they take too long in 0.5; in 0.6, I think the problem is different...

@lbenet
Copy link
Member Author

lbenet commented Mar 5, 2017

Maybe we shouldn't wait for this to be ready for 0.7...

@lbenet
Copy link
Member Author

lbenet commented Mar 5, 2017

Locally, it seems something happens with the "minimal_pow_test"... they are many, but are taking really too long.

@dpsanders
Copy link
Member

0.6 will require StaticArrays.jl after we release 0.7

I don't think the problem on 0.5 was that it took too long. It seems not to have realised that the tests finished.

@dpsanders
Copy link
Member

dpsanders commented Mar 5, 2017 via email

@lbenet
Copy link
Member Author

lbenet commented Mar 5, 2017

No. travis says that it doesn't receive any output for more than 10 min. Yet, the next item ("minimal_fma_test") is not displayed, so it was in process. It corresponds to 564 tests...

@lbenet
Copy link
Member Author

lbenet commented Mar 6, 2017

Thanks! See also #236, with similar problems...

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 91.762% when pulling 62c4b74 on lb/itf1788_Base.Test into a24a7f3 on master.

@dpsanders
Copy link
Member

The problem as usual seems to be the huge size of the pow tests.
But I don't understand why they were working and are now not working again.

@lbenet
Copy link
Member Author

lbenet commented Mar 6, 2017

Locally, they work. But they take an enormous amount of time to complete. May this be related to Base.Test?

@lbenet
Copy link
Member Author

lbenet commented Mar 6, 2017

We may still use FactCheck.jl, but implement that in parallel...

@coveralls
Copy link

coveralls commented Mar 6, 2017

Coverage Status

Coverage decreased (-11.7%) to 80.172% when pulling 48f5063 on lb/itf1788_Base.Test into a24a7f3 on master.

@lbenet
Copy link
Member Author

lbenet commented Mar 6, 2017

Why not breaking those tests in few files?

@dpsanders
Copy link
Member

Yes, breaking up the files would be the best solution. But it seems difficult to automate with ITF1788?

@lbenet
Copy link
Member Author

lbenet commented Mar 6, 2017

I agree...

@coveralls
Copy link

coveralls commented Mar 6, 2017

Coverage Status

Coverage decreased (-91.9%) to 0.0% when pulling 338239a on lb/itf1788_Base.Test into a24a7f3 on master.

@lbenet
Copy link
Member Author

lbenet commented Mar 6, 2017

I'm not sure what are you aiming...

@coveralls
Copy link

coveralls commented Mar 6, 2017

Coverage Status

Coverage decreased (-91.9%) to 0.0% when pulling 92b2b49 on lb/itf1788_Base.Test into a24a7f3 on master.

@dpsanders
Copy link
Member

dpsanders commented Mar 6, 2017 via email

@lbenet
Copy link
Member Author

lbenet commented Mar 6, 2017

What ever you did seems to have worked! You should push another commit to get the correct code-coverage
.

@dpsanders
Copy link
Member

No, it's not actually running the tests...

@dpsanders
Copy link
Member

Given the unbelievably long time that the ITF1788 tests require with Base.Teston Julia 0.5 (hopefully 0.6 will be faster), maybe we should just leave the FactCheck versions for now, and parallelize those?

I was pointed to Distributions.jl as having a simple version of parallelized tests. It is a bit different from what you did:

https://github.com/JuliaStats/Distributions.jl/blob/master/test/runtests.jl

@lbenet
Copy link
Member Author

lbenet commented Mar 6, 2017

I've just checked the way they parallelize and the sole difference is that they use pmap (parallel map) instead of @sync @parallel for. I'm not quite sure, but I think it does exactly the same.

@lbenet lbenet mentioned this pull request Mar 6, 2017
@dpsanders
Copy link
Member

What is the status of this?

@lbenet
Copy link
Member Author

lbenet commented Mar 3, 2019

I am not quite sure, but I think this is a leftover when ValidetedNumerics.jl became an umbrella package. I think we should close this and, in case of need reopen it in IntervalArithmetics.jl, where this belongs.

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.

4 participants