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

WIP Fix/m1 mac #158

Merged
merged 19 commits into from
Apr 11, 2022
Merged

WIP Fix/m1 mac #158

merged 19 commits into from
Apr 11, 2022

Conversation

dreaquil
Copy link
Contributor

Description

Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist

  • My code follows the pre-commit style and check guidelines of this project.
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing tests pass locally with my changes

@samet-akcay
Copy link
Contributor

@dreaquil, we'll need to move nncf and openvino related requirements to requirements/openvino.txt.

For the openvino imports in the inferencers, we'll first need to check if openvino is installed before importing the package. Alternatively we could import openvino in the corresponding part of the code rather than at the top of the module.

For the package version numbers we need some through testing internally to ensure that all the components work fine, and there is no performance degradation in the algorithms. After the check we could use these version numbers.

@samet-akcay samet-akcay modified the milestones: v.0.2.6, Backlog Apr 4, 2022
@samet-akcay samet-akcay linked an issue Apr 5, 2022 that may be closed by this pull request
@samet-akcay
Copy link
Contributor

@dreaquil, looks like the branch has conflicts to be solved. If you want to get the pr merged, would you be able to resolve the conflicts please? Thanks

@dreaquil
Copy link
Contributor Author

dreaquil commented Apr 5, 2022

@samet-akcay All done I believe. However, the inclusion of torchtext also created an issue to I had to upgrade it to a later version to find one compatible with m1 mac.

I cannot run some of the tests on my machine due to not having a gpu.

@dreaquil
Copy link
Contributor Author

dreaquil commented Apr 6, 2022

Hi @samet-akcay! Just checking in that the CI is still broken right?

@samet-akcay
Copy link
Contributor

@dreaquil, almost fixed. we'll try to get it up and running before the end of today.

Copy link
Contributor

@samet-akcay samet-akcay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dreaquil, these changes causes the CI to fail due linting and mypy issues. I'll try to check if we could find a common ground to install anomalib on M1 and pass the tests.

I've attached the CI logs here, showing where/why the CI fails.
CI-PR-158.log

anomalib/deploy/inferencers/openvino.py Outdated Show resolved Hide resolved
requirements/base.txt Outdated Show resolved Hide resolved
requirements/base.txt Outdated Show resolved Hide resolved
requirements/base.txt Outdated Show resolved Hide resolved
requirements/base.txt Outdated Show resolved Hide resolved
requirements/base.txt Outdated Show resolved Hide resolved
anomalib/deploy/inferencers/openvino.py Outdated Show resolved Hide resolved
requirements/base.txt Outdated Show resolved Hide resolved
requirements/base.txt Outdated Show resolved Hide resolved
requirements/base.txt Outdated Show resolved Hide resolved
@samet-akcay
Copy link
Contributor

@dreaquil, looks like black also fails. Can you run black -l 120 anomalib from your terminal, and hopefully this will be the last change. Thanks :)

if black is not installed in your virtualenv, you could run pip install black

@dreaquil
Copy link
Contributor Author

@dreaquil, looks like black also fails. Can you run black -l 120 anomalib from your terminal, and hopefully this will be the last change. Thanks :)

if black is not installed in your virtualenv, you could run pip install black

Done! here's hoping :)

@samet-akcay
Copy link
Contributor

CI has passed finally 🎉. I'm gonna be merging the PR now

@samet-akcay samet-akcay mentioned this pull request Apr 11, 2022
11 tasks
@samet-akcay samet-akcay merged commit b785f7d into openvinotoolkit:development Apr 11, 2022
@samet-akcay samet-akcay self-assigned this Apr 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Requirements incompatible with M1 Mac
2 participants