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

Fix to torch_inferencer pre_process trying to use 'to(device)' on a n… #1816

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

Sbaffinator98
Copy link

@Sbaffinator98 Sbaffinator98 commented Mar 4, 2024

Fix to torch_inferencer pre_process trying to use 'to(device)' on a n…umpy istance with broken shape

📝 Description

  • With 1.0.0 the torch inferencer is broken, when working on 501a for example, if we convert the export and inferencer of the model to the TorchInferencer we will get broken result caused by doing numpy.to(device) in the pre_process of the inferencer. Being .to(device) a tensor method it is not possible to be working.
    Provided the fix by making a .transpose of the ndarray coming in and exporting the result with torch.from_numpy(ndarray).to(device) instead of ndarray.to(device)

On my fork on branch 501a_torch_fix can be found the .ipynb with the implementation on the cubes training/inference with OpenVino but also with the TorchInferencer

✨ Changes

Select what type of change your PR is:

  • 🐞 Bug fix (non-breaking change which fixes an issue)
  • 🔨 Refactor (non-breaking change which refactors the code base)
  • 🚀 New feature (non-breaking change which adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • 📚 Documentation update
  • 🔒 Security update

✅ Checklist

Before you submit your pull request, please make sure you have completed the following steps:

  • 📋 I have summarized my changes in the CHANGELOG and followed the guidelines for my type of change (skip for minor changes, documentation updates, and test enhancements).
  • 📚 I have made the necessary updates to the documentation (if applicable).
  • 🧪 I have written tests that support my changes and prove that my fix is effective or my feature works (if applicable).

…umpy istance with broken shape

Signed-off-by: AndreaDor <dorigoandrea98@gmail.com>
@Sbaffinator98
Copy link
Author

Thie is my first time asking for a pr on any open source project, i am open to correct and explain what i did. Thank you

@samet-akcay
Copy link
Contributor

Thie is my first time asking for a pr on any open source project, i am open to correct and explain what i did. Thank you

Welcome to the community, @Sbaffinator98! Thanks a lot for your contribution!

I'll be running the tests now, and we will be reviewing your PR shortly. Thanks again!

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.

Hi @Sbaffinator98, thank you for addressing this. I've added some suggestions to fix the code-quality checks. If you want to re-produce it on your side, you could do the following from your terminal:

pip install pre-commit
pre-commit run -a

src/anomalib/deploy/inferencers/torch_inferencer.py Outdated Show resolved Hide resolved
src/anomalib/deploy/inferencers/torch_inferencer.py Outdated Show resolved Hide resolved
src/anomalib/deploy/inferencers/torch_inferencer.py Outdated Show resolved Hide resolved
@samet-akcay samet-akcay added Bug Something isn't working Requires Changes Reviewers request changes, which should be addressed by the PR maker labels Mar 18, 2024
@Sbaffinator98
Copy link
Author

Hello i saw that you guys commited some clean-code. I see that is still failing the checks. I dont see any torch_inferencer related issue. What should i do further?

@samet-akcay
Copy link
Contributor

Hello i saw that you guys commited some clean-code. I see that is still failing the checks. I dont see any torch_inferencer related issue. What should i do further?

The tests are failing for some reason here
https://github.com/openvinotoolkit/anomalib/actions/runs/8367192205/job/22909054968?pr=1816

I'll check why they fail

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Requires Changes Reviewers request changes, which should be addressed by the PR maker
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants