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

dockerization #2

Closed
BBO-repo opened this issue Dec 22, 2023 · 6 comments
Closed

dockerization #2

BBO-repo opened this issue Dec 22, 2023 · 6 comments

Comments

@BBO-repo
Copy link

Hello,
Thanks for your repo! You really did what you said there.
Some suggestions that I have is that your repo is too much dependent on your local computer configuration, I thought a Dockerfile to play with your repo would be nice to have.
If you think it is valuable to add it I can add my Dockerfile, it automatically gets all dependencies: get libtorch and build latest OpenCV and build the application.

Also I've tried your code with get but got an error, seems that some tensor are in CPU while other are in GPU. Let's dig this point later
I would have some C++ specific comments on the code but let it for later, a PR was requested to solve a linux build.

@ksasso1028
Copy link
Owner

ksasso1028 commented Dec 25, 2023

thank you for your comments and input !
RE: device pinned- because I had to trace the model, the device is pinned to the device it was traced on. I can upload GPU pinned weights? I know it seems counterintuitive as it breaks the logic of the TorchModel class, which supports moving between devices with scripted models. I know it works on the CPU (traced on CPU) , so let me know if that is broken for you. I can take a second look at scripting the models as that would allow the modularity of the TorchModel class.

RE:docker - this would be great ! I can add this either way, as it seems important for others to get up speed, released this on a whim and certainly could use some cleaning up for production. Feel free to make a PR

Gonna take a look at your PR, thank you for your contribution and input !

@BBO-repo
Copy link
Author

Hello,
No problem, happy to help on this topic!
If I can have it work properly on my machine I think I could go on helping in C++ code and C++ porting some other Python EasyOCR features in this repo after validating with you.

RE: docker, I'll send a PR soon.
RE: device pinned, can I add another issue to precisely address this, to avoid blurring the current discussion.

@ksasso1028
Copy link
Owner

Great, just merged your branch

@BBO-repo
Copy link
Author

BBO-repo commented Jan 1, 2024

Hi,
I've added the PR for dockerization.
I can not test on Windows, I hope you would be able to test on Windows to confirm it's still working for you, and also on Docker.
Afterward, I would also have some comments to avoid users to manually modify the cmakelists.txt to have their build working on Windows.

@ksasso1028
Copy link
Owner

sounds good, just getting around to this will check it out today.

@ksasso1028
Copy link
Owner

merged a bit ago, also added some changes to the decoding for better results, cheers

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

No branches or pull requests

2 participants