10000 CRON Build breaks · Issue #10074 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]

Skip to content

CRON Build breaks #10074

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

Closed
massich opened this issue Nov 6, 2017 · 26 comments
Closed

CRON Build breaks #10074

massich opened this issue Nov 6, 2017 · 26 comments

Comments

@massich
Copy link
Contributor
massich commented Nov 6, 2017

The cron build of travis appears to break master (see) which actually does not (see the same commit). The thing is that in the cron build uses numpy 1.14.0.dev0+d607099 whereas the regular build uses numpy 1.13.1-py36h5bc529a_2. This makes doctests to break due to precission.

Expected:
    0.824...
Got:
    0.825

ping @lesteve

@TomAugspurger
Copy link
Contributor

I think numpy/numpy#9966 is fixing this on NumPy's end (it hit pandas too).

@lesteve
Copy link
Member
lesteve commented Nov 6, 2017

Thanks @TomAugspurger.

For the record, the cron job is supposed to test the numpy dev version. I was expecting that we would get an email if the cron job failed, and I did not, not sure why ...

@jnothman
Copy link
Member
jnothman commented Nov 6, 2017 via email

@amueller
Copy link
Member
amueller commented Nov 6, 2017

Can we add documentation about the cron build to the dev docs please? I don't really understand how it is set up. And I didn't get an email from what I can see :-/

@lesteve
Copy link
Member
lesteve commented Nov 7, 2017

Can we add documentation about the cron build to the dev docs please? I don't really understand how it is set up.

I'll try to do that. In the mean time:

  • the definition of what's get run in the CRON job is done in the .travis.yml, exactly the same way as the other Travis job, except that you have a type = cron filter. See

    scikit-learn/.travis.yml

    Lines 58 to 63 in abb43c1

    # This environment tests scikit-learn against numpy and scipy master
    # installed from their CI wheels in a virtualenv with the Python
    # interpreter provided by travis.
    - python: 3.6
    env: USE_PYTEST="true" DISTRIB="scipy-dev-wheels"
    if: type = cron
    .
  • which branch is targetted and the frequency of the cron job is set via the web UI at https://www.travis-ci.org/scikit-learn/scikit-learn/settings.

More information about Travis cron jobs: https://docs.travis-ci.com/user/cron-jobs/

@qinhanmin2014
Copy link
Member

Really curious what's happening recently. Seems that almost everyday, successful Travis builds are marked as cron, run again by Travis and marked as fail on Master. See for example:
2017-11-13_110400
2017-11-13_110418

@jnothman
Copy link
Member

CRON builds use the dev version of scipy, numpy, which is what it is intended to test, but at the moment that is failing due to numpy/numpy#9966

@jnothman
Copy link
Member
jnothman commented Nov 13, 2017

Also note that that PR was merged 14 hours ago, and the last cron job was run 20 hours ago... we'll see if perhaps we get a green tick overnight, or if we need to adjust our doctests.

@qinhanmin2014
Copy link
Member

@jnothman Thanks for the instant reply. I think I'll quickly push a doc fix to master to see if everything goes well.

@jnothman
Copy link
Member

What fix is needed, unless you have run tests with numpy master now? It's best to make a PR in any case, to make sure that master builds on numpy stable too.

@qinhanmin2014
Copy link
Member

Sorry, by saying push, I mean a PR (See #10123, a broken link in doc). And I suddenly realize that it seems that I don't need to do that since the cron job will automatically run again. Sorry again for doing all the things in a hurry. I promise that I won't commit anything directly into master.

@jnothman
Copy link
Member
jnothman commented Nov 13, 2017 via email

@jnothman
Copy link
Member

(Also, of course, if you're absolutely certain they would not be the subject of disagreement.)

@lesteve
Copy link
Member
lesteve commented Nov 13, 2017

Still breaking as of now (finished 40 minutes ago). There were a couples of PRs in numpy that were merged very recently so I am hoping they were not in the latest numpy dev wheels and everything is going to be fine tomorrow.

@qinhanmin2014
Copy link
Member

There were a couples of PRs in numpy that were merged very recently so I am hoping they were not in the latest numpy dev wheels and everything is going to be fine tomorrow.

This time, we got an errored build. See https://travis-ci.org/scikit-learn/scikit-learn/builds/301820919

@lesteve
Copy link
Member
lesteve commented Nov 14, 2017

According to https://github.com/numpy/numpy/pull/9332/files, looks like we need legacy=True rather than sign='legacy'. I'll do that.

@qinhanmin2014
Copy link
Member

ping @jnothman
It seems that latest numpy changes (including numpy/numpy#9966) do not fix any error in the cron job. For today, the cron job get errored (See log) and @lesteve has fixed it (enable the cron job to run through again) in #10132 but all the errors are still there.
For the failed tests, see
cron job in master yesterday: log
cron job ran by @lesteve today: log
Also, since the failed tests are all doc tests, seems that it can be solved by using "# doctest: +SKIP", but I'm not sure when to use it in scikit-learn.

@lesteve
Copy link
Member
lesteve commented Nov 14, 2017

Hmmm then not sure what is going on ... wild guessing: maybe the np.set_printoptions(legacy=True) prevents numpy/numpy#9966 from having an effect somehow.

I think the first thing to do would be to build numpy master locally and see whether we can reproduce the errors.

@qinhanmin2014
Copy link
Member

@lesteve personally I might doubt whether numpy/numpy#9966 is the reason, I've tried to locate a series of commit in numpy by looking at the latest successful cron build and the first failed cron build, l'd rather believe numpy/numpy#9941 to be the reason but l'm not sure.

@qinhanmin2014
Copy link
Member

ping @lesteve @jnothman
The same 9 doctests fail today log, maybe something to do in scikit-learn?
last successful cron job log
numpy version: numpy 1.14.0.dev0+8b9195b
first fail cron job log
numpy version: numpy 1.14.0.dev0+b97f9b0
The most suspectable PR : numpy/numpy#9941

@lesteve
Copy link
Member
lesteve commented Nov 15, 2017

I think there are mostly two things here:

/home/travis/build/scikit-learn/scikit-learn/sklearn/linear_model/coordinate_descent.py:897: DocTestFailure
____________ [doctest] sklearn.metrics.pairwise.manhattan_distances ____________
498         If sum_over_features is False shape is
499         (n_samples_X * n_samples_Y, n_features) and D contains the
500         componentwise L1 pairwise-distances (ie. absolute difference),
501         else shape is (n_samples_X, n_samples_Y) and D contains
502         the pairwise L1 distances.
503 
504     Examples
505     --------
506     >>> from sklearn.metrics.pairwise import manhattan_distances
507     >>> manhattan_distances([[3]], [[3]])#doctest:+ELLIPSIS

Expected:
    array([[ 0.]])
Got:
    array([[0.]])

All in all, we can solve part of it with doctest ellipsis which could be a good middleground but not all of them, eg not this one:

/home/travis/build/scikit-learn/scikit-learn/sklearn/metrics/regression.py:233: DocTestFailure
_______ [doctest] sklearn.metrics.cluster.supervised.completeness_score ________
411       >>> from sklearn.metrics.cluster import completeness_score
412       >>> completeness_score([0, 0, 1, 1], [1, 1, 0, 0])
413       1.0
414 
415     Non-perfect labelings that assign all classes members to the same clusters
416     are still complete::
417 
418       >>> print(completeness_score([0, 0, 1, 1], [0, 0, 0, 0]))
419       1.0
420       >>> print(completeness_score([0, 1, 2, 3], [0, 0, 1, 1]))
Expected:
    1.0
Got:
    0.9999999999999999

@lesteve
Copy link
Member
lesteve commented Nov 15, 2017

I have opened numpy/numpy#10026 for the space issue with arrays of size 1.

@qinhanmin2014
Copy link
Member

ping @jnothman @lesteve
Surprising that cron job suddenly passes today. See log. Closing this though I can't figure out the reason.

@lesteve
Copy link
Member
lesteve commented Nov 21, 2017

Good, I don't really understand either. FWIW all the tests were passing on my machine when I tried to reproduce. Anyway let's assume it is fixed until it fails again ...

Side-comment, something to bear in mind AFAIK is that the numpy dev wheel is published by a Cron daily job which means that the numpy dev wheel is lagging behind numpy master a bit.

@mohamed-ali
Copy link
Contributor

Apparently, this error is still there.

Expected:
    array([ 13. ,  9.5 ,  12. ,  17. ])
Got:
    array([ 13. ,   9.5,  12. ,  17. ])

Here's the log where I am seeing it:

@lesteve
Copy link
Member
lesteve commented Mar 26, 2018

This is very unlikely that this bug is still here. ping me in your PR if you can not figure it out and need help in fixing the Travis failures.

Note that right now if you are using numpy >= 1.14, you need to set_printoptions(legacy='1.13') when you generate the expected doctests results. This is a bit annoying and tackled in #10835.

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

No branches or pull requests

7 participants
0