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

Alternatives to opencv (cv2) #532 [Solved] #853

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Dilip-Jain
Copy link

@Dilip-Jain Dilip-Jain commented Oct 1, 2023

Issue No. #532 [Solved]

Problem

@jklaise mentions -

opencv is a fairly heavy dependency and requires building (see recent CI failures), but we only use it in one place for making image perturbations (https://github.com/SeldonIO/alibi-detect/blob/master/alibi_detect/utils/perturbation.py). We should either explore replacing it with another library that's lighter (potentially scikit-image) or make it an optional dependency.

Solution

Hello maintainers and contributors,

I am pleased to submit this pull request which addresses the concern of OpenCV being a heavy dependency in the image perturbations module (alibi_detect/utils/perturbation.py). The requirement of cv2 (opencv), as realized, was limited and eliminable as it was used only for making image perturbations. In this pull request, I have replaced specific OpenCV functionalities with the lighter and already required libraries, scikit-image and SciPy, ensuring that no new external libraries are introduced.
Fixes #532

Key Replacements:

  1. Replaced OpenCV's GaussianBlur and filter2D with their counterparts from SciPy.
  2. Substituted OpenCV's affineTransform and warp functions with the corresponding functionalities from scikit-image.
  3. Modified setup.py: Updated the setup.py file to make the opencv-python requirement optional, allowing users to install it based on their needs.
  4. Handled cv2 Import Dynamically: Modified the code in perturbation.py to handle the cv2 import using a try-except block. This ensures that if OpenCV is installed, it is used, but the code remains functional without OpenCV as well.

Benefits of the Changes:

  • Reduced Dependency Size: By replacing OpenCV with scikit-image and SciPy, we significantly reduce the overall library size, making the project more lightweight.
  • Simplified CI Pipeline: The elimination of OpenCV as a dependency simplifies the continuous integration (CI) process and mitigates potential CI failures related to building and managing OpenCV.
  • Improved Accessibility: Utilizing scikit-image and SciPy, which are already required libraries, enhances accessibility for developers, eliminating the need to manage an additional heavyweight dependency. With optional OpenCV dependency, users now have the option to install OpenCV (opencv-python) based on their specific use cases, preventing unnecessary bloat for those who don't require it.
  • Future-Proofing: The try-except import handling allows for seamless transitions in case OpenCV becomes a necessity in the future, minimizing code changes and maintaining backward compatibility.
  • Enhanced Flexibility: Developers who already have OpenCV installed can continue to leverage its functionality within the image perturbation module without any disruption.
  • The changes in this pull request ensure compatibility and alignment with the project's objectives, maintaining high-quality image perturbations while minimizing dependencies.

Thank you for considering this pull request, and I welcome any feedback or suggestions for further improvements.

@jklaise
Copy link
Member

jklaise commented Oct 3, 2023

Hi @Dilip-Jain and thanks for your contribution, this looks very helpful!

I would suggest removing the cv2 option completely as I don't see a need for it if we can achieve the same functionality with scipy and skimage, it also reduces the burden on supporting another dependency.

Have you had a chance of running some of the example notebooks that use these transforms and see if it all works as expected? E.g. grepping for defocus_blur in doc/source/examples gives about 6 notebooks where these are used.

@Dilip-Jain
Copy link
Author

Hi @jklaise,
Most (all) the example notebooks with defocus_blur or elastic_transform were corruptions for the cifar10c dataset so testing one for all was enough. However, only after around half an hour, I realized these "corruptions" weren't even using perturbation but rather picking up .npy objects from your Google storage. 🤦‍♂️

Regardless, while making modifications, I compared the outcomes of the previous and the updated code using random images and calculated the Mean Squared Error (MSE) and Structural Similarity Index (SSI). Both scores were close to 1, indicating highly similar outputs. In fact, based on these scores, I decided which implementation to use—whether it was scipy, skimage, or even a custom numpy convolution—to replace the (then) existing code.

Lastly, just so we are on the same page, the current commit has cv2, but only as an optional dependency and you want me to remove it in its entirety. The next commit (updated pull request ⬇️) will do exactly that! 😊
Doubt: what about cv2 (opencv) mentioned in conf.py and license!!

@jklaise
Copy link
Member

jklaise commented Oct 12, 2023

@Dilip-Jain apologies, I somehow missed your latest comment. I've also realized that the functions are not actually used to produce runtime corruptions either in the codebase or the examples.

I'm currently looking at the PR and testing it myself (basically checking if outputs are the same using np.testing.assert_allclose). I hope you don't mind if I push the removal of cv2 to this branch? Then we should be able to get this change in this week.

@Dilip-Jain
Copy link
Author

Dilip-Jain commented Oct 13, 2023

Sure @jklaise,
Let me know if something needs to be changed or if I can help you with anything. By the time you complete the testing, I'll remove the cv2 (currently, is as an alternative/option) entirely.

@Dilip-Jain
Copy link
Author

Hi @jklaise,
Any progress on this?

@CLAassistant
Copy link

CLAassistant commented May 7, 2024

CLA assistant check
All committers have signed the CLA.

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.

Alternatives to opencv (cv2)
3 participants