-
-
Notifications
You must be signed in to change notification settings - Fork 25.1k
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
updated examples and tests that use scipy.misc.lena() #5920
updated examples and tests that use scipy.misc.lena() #5920
Conversation
…removed in scipy version 0.17, to use scipy.misc.face() instead.
It seems like the tests on circleCI and travis-CI are failing because they don't have the version of scipy needed to access the face image (v 0.12). How should I be getting around this? |
@GaelVaroquaux sorry if i was unclear, but it does work with version 0.12 (that's the version during which the face image was added [see: https://github.com/scipy/scipy/pull/351] ) . travisci / circleci are running earlier versions. |
@GaelVaroquaux sorry if i was unclear, but it does work with version 0.12
OK. That's great news!
|
I would rather upgrade the circleci configuration to have a more recent version of scipy to build the examples. I think it's ok if some examples are broken on very old scipy (as 0.9) as long as the tests pass and the examples run correctly on more recent scipy. |
I agree. Remain the issue that the segmentation example takes forever. Maybe that |
@nelson-liu how long does it take to run this example on your machine if you install PyAMG? Maybe we could skip this example if PyAMG is not installed? |
I think we should just raise a SkipTest in this one test if scipy is too old. I want travis running old scipy. |
@amueller how would I raise a skiptest? Or is that not something I should fiddle with. |
+1 @nelson-liu use |
To speed up the computation you can try to resize the image, e.g. to resize to 25%: face = sp.misc.imresize(face, 0.10) and also play with the |
Actually increasing the tolerance of the arpack solver yields to poor results. I got better results by twicking the other parameters of the example:
It runs in ~8s total on my laptop without pyamg. Here are the results: |
@ogrisel that actually doesn't look too bad! Shall I keep tweaking the parameters, or should we go with that diff? |
Feel free to reuse the parameters I gave in the diff. If you tweak further and find more interestingly looking results that take less than 20s to compute, feel free to use them in your PR. |
@nelson-liu if you don't have the time to finish addressing the comments of this PR, let me know and I can takeover from here. |
@ogrisel, sorry I've just been busy lately with the holiday season and that's mostly passed. I'll get this example into the pr by tomorrow night. Was a decision ever reached as for scipy versions for circleci and travisci? |
…n, dramatically improving runtime of the example.
@ogrisel I've added your changes to the PR. I feel like that is the last code-based change left? The only thing is to decide how to deal with circleci / travisci in terms of them running older versions of scipy that do not have the face() image. |
As @amueller said, For circle ci we need a more recent version of scipy, either by using an apt repository such as neurodebian that provides recent packages for numpy and scipy on old distributions such as ubuntu 12.04 (see http://neuro.debian.net/) or by find a way to configure circle ci to use a more recent distribution such as ubuntu 14.04 or by using anaconda instead of the system packages. |
As of now changing the version of the base image of circle ci is not yet possible: https://discuss.circleci.com/t/when-are-we-going-to-get-ubuntu-14-04/331/4 So the best bet is either to use miniconda (as done in the travis configuration) or by leveraging the neurodebian repo to add to the ubuntu 12.04 distro used in the base cicle ci image. |
@ogrisel sorry I'm not very familiar with unit testing on travis, would I just do something like this: http://stackoverflow.com/questions/30867145/tell-travis-to-skip-a-test-but-continue-to-include-it-in-my-main-test-suite/30874941#30874941 edit: rather, should I just raise a skiptest on the version of scipy imported instead of specifically targeting travis? |
No have a look at other example usage of SkipTest in scikit-learn by using git grep as I said earlier. |
Yes, exactly. |
Ah apologies, didn't see your comment about git grep. thanks! |
- sudo apt-get update | ||
- sudo apt-get install libatlas-dev libatlas3gf-base | ||
- sudo apt-get install build-essential python-dev python-setuptools | ||
- sudo apt-get install python-numpy python-scipy python-dev python-matplotlib | ||
- sudo apt-get install python-nose python-coverage | ||
- sudo apt-get install python-sphinx | ||
- pip install cython |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is no longer needed as it can be installed via conda directly.
edit: I didn't read your previous comment before posting this. I'll implement the changes you highlighted in the diff and see what happens. Thanks for the input!
I enabled an ssh-build, and ssh'd into the machine after the build failed. The venv-system of circleci was still active. I deactivated the venv-system, activated testenv, and ran a conda list. This command ran successfully, and all the dependencies were there. Then, from within testenv, I ran setup.py install --user, which also went off without a hitch. In summary, I believe its an issue with not properly deactivating the venv that comes with circleci. However, it could simply just be that the venv-system is automatically activated upon ssh (I logged out and logged back in and it was active again). It could possibly be related to this: https://discuss.circleci.com/t/disable-autodetection-of-project-or-application-of-python-venv/235 Any thoughts? |
Indeed, it might be a good idea to get rid of it. Creating a conda env from within a virtualenv seems to cause the "conda command not found" problem and can probably cause other issues. |
@ogrisel is there any way to go about getting rid of it completely?
Should Pillow be a dependency? In the file (gen_rst.py) it has:
|
Please add the |
It should be possible to just delete the virtualenv folder. Calling |
hmm, make html seems to have segfaulted. Here's the full trace
Any ideas as to why this might be happening? |
so I ssh'd into the box after the segfault, and tried to run make html. However, it would segfault on the stock market example. I also tried to run the script directly, and got a segfault. Could this possibly be related to: #5724? |
@ogrisel not sure we want to include this in the release? |
Yeah that would be great. Need to fix the doc build configuration though. |
closing in favor of the squashed and rebased version at #6260. |
scipy.misc.lena() will be removed in scipy version 0.17, so this PR changes all tests and examples that use it to use scipy.misc.face() instead. Addresses issue #5739. I've verified that all the scripts work on my local machine, except for what was formerly plot_lena_segmentation.py (now plot_face_segmentation.py). plot_face_segmentation.py runs extremely slowly; I ran it for around 6 hours during as I was running "make html" before I gave up and interrupted it. This is probably related to bug #1966.