Skip to content
This repository has been archived by the owner on Sep 18, 2024. It is now read-only.

Get rid of scipy #322

Open
2 tasks done
eli-osherovich opened this issue Nov 1, 2020 · 12 comments
Open
2 tasks done

Get rid of scipy #322

eli-osherovich opened this issue Nov 1, 2020 · 12 comments
Labels
image Related to images

Comments

@eli-osherovich
Copy link
Contributor

Please make sure that the boxes below are checked before you submit your issue.
If your issue is an implementation question, please ask your question on StackOverflow or on the Keras Slack channel instead of opening a GitHub issue.

Thank you!

  • Check that you are up-to-date with the master branch of keras-preprocessing. You can update with:
    pip install git+git://github.com/keras-team/keras-preprocessing.git --upgrade --no-deps

  • Provide a link to a GitHub Gist of a Python script that can reproduce your issue (or just copy the script here if it is short).

From my experience, relying on SciPy for image transformation makes it:
a) slow
b) non-suitable for TPU

Is there any particular reason to keep SciPy in the loop?

@eli-osherovich eli-osherovich added the image Related to images label Nov 1, 2020
@Stanfording
Copy link

Stanfording commented Jan 14, 2021

For the m1 apple chip, SciPy is not available yet, but the ImageDataGenerator.flow_from_directory requires it. So getting rid of scipy would be a temporary relief for the m1 users. Maybe not necessary though ( since SciPy is working on it too I guess).

@eli-osherovich
Copy link
Contributor Author

I can do this if there is a real need from the keras side.

@Dref360
Copy link
Contributor

Dref360 commented Jan 14, 2021

We use scipy here for SVD (I think np.linalg.svd is called?)
here

And we use scipy.ndimage for matrix multiplication here:
ndimage.interpolation.affine_transform.

Do you think it would require a lot of work to replicate affine_transform behaviour?

@eli-osherovich
Copy link
Contributor Author

eli-osherovich commented Jan 15, 2021 via email

@Dref360
Copy link
Contributor

Dref360 commented Jan 15, 2021

PRs welcome then :)

@eli-osherovich
Copy link
Contributor Author

eli-osherovich commented Jan 15, 2021 via email

@Stanfording
Copy link

I can do this if there is a real need from the keras side.

Yes, please! Because scipy is important to Keras and the machine learning community.

@eli-osherovich
Copy link
Contributor Author

eli-osherovich commented Jan 18, 2021

Yes, please! Because scipy is important to Keras and the machine learning community.

Just to make sure: the idea is to remove scipy dependency from keras. Namely, it will not make scipy working on M1, we'll just replace it with something else so as to let keras be used on M1.

@Stanfording
Copy link

Yes, please! Because scipy is important to Keras and the machine learning community.

Just to make sure: the idea is to remove scipy dependency from keras. Namely, it will not make scipy working on M1, we'll just replace it with something else so as to let keras be used on M1.

That's exactly what I meant!

@eli-osherovich
Copy link
Contributor Author

@Stanfording first patch is waiting for approval, as you can see. I chose to replace scipy with numpy, but I cannot test it on M1.... Actually, after thinking about it a bit, I do not know if numpy is working on M1...

@Stanfording
Copy link

@Stanfording first patch is waiting for approval, as you can see. I chose to replace scipy with numpy, but I cannot test it on M1.... Actually, after thinking about it a bit, I do not know if numpy is working on M1...

There are some issues with numpy if just downloading by itself. However, numpy works with tensorflow if downloading tensorflow with this link https://github.com/apple/tensorflow_macos . So many people who cannot download numpy on m1 just download the TensorFlow from the link above, which will install numpy automatically. The version of this numpy is currently 1.18.5. Python version is 3.8.2 from xcode command line tool. The way to use keras on m1 is mostly use "from tensorflow import keras" instead of downloading keras directly and just import keras. So if there is any update from keras, we can only use it from the one imported from tensorflow. I hope the information helps.

@Stanfording
Copy link

Stanfording commented Jan 19, 2021

@Stanfording first patch is waiting for approval, as you can see. I chose to replace scipy with numpy, but I cannot test it on M1.... Actually, after thinking about it a bit, I do not know if numpy is working on M1...

However, it is possible for m1 users to download keras directly. But it's easy to run into many issues. I spent a lot of time to solve issues and finally get keras downloaded successfully in the same virtual environment with tensorflow. However, when using it, I could just type "python3" and "import keras" on the terminal in the virtual environment, which imported successfully, but if I import keras in a text editor (spyder) and use "python3 xxx.py" in virtual environment, it will say "No module named 'keras'". If this problem is impossible to solve by users, then the only way we can get keras is this link: https://github.com/apple/tensorflow_macos. I hope this is also helpful.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
image Related to images
Projects
None yet
Development

No branches or pull requests

3 participants