Repository navigation
Conversation
|
Label error. Requires exactly 1 of: changelog:.*. Found: 🧠machine-learning. A maintainer will add the required label. |
|
Please fill out the PR description. |
|
The description is the first post, at the top of the page. You can edit it by clicking the three dots. |
|
OK,I did. |
mertalev
left a comment
There was a problem hiding this comment.
Thanks for the PR! It’s a step in the right direction, but I think the strategy needs refinement and the code quality could be improved as well.
| resize_h = int(round(resize_h / 32) * 32) | ||
| resize_w = int(round(resize_w / 32) * 32) | ||
|
|
||
| # Calculate target size (fixed shape for padding) |
There was a problem hiding this comment.
Instead of using a square, I think a boxing strategy for different aspect ratios would be better to avoid excessive padding. Also, the logic here is flawed. It clamps to target resolution even if it isn't a multiple of 32.
There was a problem hiding this comment.
I think this is not ideal because using multiple shapes would further increase the memory usage. Currently, a graphics card with at least 5GB of VRAM is required to run it. If this continues, the requirements are too high, and many small servers might not be able to run it. Alternatively, we could consider using an environment variable to define this.
There was a problem hiding this comment.
There would only be a handful of variations. I think the main ones would be 16:9, 1:1 and 9:16 vs just 1:1. The behavior on main allows any number of different box dimensions, so having a few shapes is still much better than current.
There was a problem hiding this comment.
Yes, theoretically, this would improve the accuracy rate, but how should we verify it through practical testing? If possible, I'll test it this weekend.
There was a problem hiding this comment.
The best way would be to see if/how the results change with different input images, comparing main and the PR.
Once you can confirm they're the same (or very close), you can check the README for testing with Locust. It simulates concurrent requests to the ML server, which lets us see both the latency/throughput impact of the change and how memory usage is affected. The locustfile.py only creates a random image though, so you might need to make it use a real image with text for the test. It depends on how far you want to go with it, but you can also make it use different images to test the aspect ratio handling as well (otherwise, it will only be testing a static input dimension).
There was a problem hiding this comment.
I mean that the recognition rate currently appears normal, and changing the logic shouldn't have much impact.
There was a problem hiding this comment.
The amount of padding affects speed (the less similar the image is to a square, the more compute is wasted on padding). On CPU, it would increase memory usage. I'm not too worried about accuracy since padding shouldn't change much there (assuming the padding value is actually correct).
There was a problem hiding this comment.
From my testing. The current performance is normal. With an i5-10400F and a 1660 SUPER, it processed 14,107 photos in less than two hours.CPU usage is at 9%.The graphics card is running at full power.
| normal_indices = [] | ||
| extreme_indices = [] | ||
|
|
||
| EXTREME_WH_RATIO = 12.0 |
There was a problem hiding this comment.
Similarly, it could box into aspect ratios in a cleaner way without hardcoded “normal” and “extreme” paths. It should probably also be padded to fixed aspect ratios rather than padding to any max, but it’s a bit tricky since RapidOCR handles part of the processing here.
There was a problem hiding this comment.
Since the RapidOCR project was not primarily designed with ONNX in mind, some of its code is not suitable. If you don't want this to be the case, you might need to modify the RapidOCR library.The main problem here is each time you use RapidOCR, it only aligns with the maximum value of that instance. When it is called next time, it does not have the previous parameters, and the shape will change. Therefore, we must ensure that the input shape is the same each time.
|
I tried this, but unfortunately memory still leaks during 10 minutes of running with Intel Arc Pro. In my case I think this is some ONNX Runtime leak though unfortunately. |
Perhaps ONNX also has issues, as I encountered an unusual memory usage of about 500M during testing, but it was extremely rare, and I couldn't find a way to reproduce it. Could you share the problematic photo? |
|
I believe it's another issue which is even worsened by #23462. It just monotonically increases until both graphics and system memory will fill up and comes OOM errors from ONNX. Memory usage increases exponentially, slowly at first and then it explodes. |
|
This is look like that. |
|
I see this as a steady growth, it should not be a problem with the image, but I still hope We can try using the Nvidia environment to see if my modifications only work for the CUDA environment. I only have a CUDA environment and cannot reproduce your issue. |


Description
This PR fixes excessive GPU memory usage in the OCR pipeline.
These changes stabilize GPU memory usage while preserving OCR accuracy.
Fixes #23462
How Has This Been Tested?
Tested locally using the default Immich OCR configuration with ONNX Runtime.
Checklist:
Please describe to which degree, if any, an LLM was used in creating this pull request.
An LLM was used as a general coding and reasoning assistant during development.