[New] Consistent usage of "similarity" and "confidence"

[adombeck]

Description

This PR contains multiple improvements to the OCR modules used by yarf:

1. Consistently use the terms "confidence" and "coincidence", and use percentage for both
2. Make yarf/vendor/RPA/recognition/ocr.py return actual confidence
3. Log the matched text in find_text
4. Log rejected matches with high similarity
5. Log image which matched text

See commit messages for details.

Tests

The existing tests were updated and run via uv run pytest.

[adombeck]

While testing the changes to yarf/vendor/RPA/recognition/ocr.py I noticed how much worse its results are compared to RapidOCR. Why do we even keep it? It would simplify things quite a lot if we could just remove it and always use RapidOCR.

[p-gentili]

Hey @fernando79513, can you please take a look at this PR? The idea looks good to me but you worked on those files in the first place and you might have a different view on it.

[fernando79513]

While testing the changes to yarf/vendor/RPA/recognition/ocr.py I noticed how much worse its results are compared to RapidOCR. Why do we even keep it? It would simplify things quite a lot if we could just remove it and always use RapidOCR.

The only reason we kept this is because rapidOCR is noticeably slower than tesseract (around 2x), specially on big images. We may want to keep it in case we need faster detection times.

Nevertheless, maintaining both introduces some complexity, so I don't have a strong opinion about keeping Tesseract. Any comments on that @p-gentili?

[fernando79513]

Thanks a lot for the work here.
I usually prefer having confidence and similarities with values from 0-1 instead of 0-100. We didn't change that in the beginning because we couldn't modify the RPA library, but I guess it should be possible now that it's vendorized. I leave it up to you though.

[p-gentili]

Nevertheless, maintaining both introduces some complexity, so I don't have a strong opinion about keeping Tesseract. Any comments on that @p-gentili?

Although I believe tesseract just doesn't work well in our use cases, this change doesn't really justify introducing a breaking change. The patch doesn't look that complex, and it would be similar to the amount of work required for any other OCR engine we might introduce.

Nevertheless, we can discuss separately about the removal, but I would keep this PR as is.

[adombeck]

I usually prefer having confidence and similarities with values from 0-1 instead of 0-100.

@p-gentili said in #100 (comment) that he prefers 0-100, so that's what I implemented here. I don't have an opinion on that so I'll let you two discuss it.

[adombeck]

Rebased on main and resolved the conflicts.

[adombeck]

I noticed that a call to get_text_position now logs the matching image twice, because it calls match_text (which calls find_text, where we log the image now), and the function logs it itself in:

yarf/yarf/rf_libraries/libraries/video_input_base.py

Line 404 in a04652e

log_image(matched_image, "Matched text region:")

I'll remove the log_image call in get_text_position to avoid that.

[p-gentili]

@p-gentili said in #100 (comment) that he prefers 0-100, so that's what I implemented here. I don't have an opinion on that so I'll let you two discuss it.

lol, let's fight @fernando79513 ! I like integers, that's all. If in this context [0,1] makes more sense that's fine by me.

[fernando79513]

Just a minor comment, but I think it would make more sense to have the logs like this.
Let me know what you think. It's not really impactful for the code execution.

[fernando79513]

If you are having a similarity threshold, why not having also a confidence threshold?
Also, why is the similarity log threshold the same as the similarity threshold?
I think it would be more useful to log the cases in which both confidence and similarity are close to the threshold values:

Suggested change

	elif similarity >= self.SIMILARITY_LOG_THRESHOLD:
	logger.debug(
	f"Rejected match for text '{match_text}' "
	f"with similarity {similarity} "
	f"and confidence {item.confidence}: '{item.text}'"
	)
	elif (
	similarity >= self.SIMILARITY_LOG_THRESHOLD
	and item.confidence >= self.CONFIDENCE_LOG_THRESHOLD
	):
	logger.debug(
	f"Rejected match for text '{match_text}' "
	f"with similarity {similarity} "
	f"and confidence {item.confidence}: '{item.text}'"
	)

I'd suggest:

• SIMILARITY_LOG_THRESHOLD = 70.0
• CONFIDENCE_LOG_THRESHOLD = 60.0

Another option would be having just a "LOG_THRESHOLD" and compare:

            elif (
                similarity >= similarity_threshold - self.LOG_THRESHOLD
                and item.confidence >= confidence_threshold - self.LOG_THRESHOLD
            ):

[adombeck]

If you are having a similarity threshold, why not having also a confidence threshold?

I don't think that would be useful. The confidence score measures how confident the OCR engine is that the string it returned is the actual string that's on the image. Let's say we want to find the string "foo" in an image, but the image only contains "bar". The OCR engine returns {"text": "bar", "confidence": "99"}, i.e. it's confident that the image really contains "bar". We calculate the similarity to "foo" which results in a similarity score of 10. Logging the message Rejected match for text 'foo' with similarity 10 and confidence 99: 'bar' is not very helpful. Does that make sense?

Also, why is the similarity log threshold the same as the similarity threshold?

Because I only found it useful when setting a higher threshold (which is made possible by #100). The log message is only useful to spot false negatives (i.e. a match was rejected even though it should have been accepted). We found the default threshold to be so low that it frequently causes false positive matches, so I doubt that there will be a lot of false negatives with an even lower similarity score.

[fernando79513]

I don't think that would be useful

But in that case you won't log anything. Both have to be close to the threshold to log anything.
Imagine you text is blurry, but the OCR still manages to get your word right. You try to look for foo, and you get: {"text": "foo", "confidence": "65"}. The OCR was close to get it, but the confidence was just below the threshold. I think that information could be useful.

If the OCR returns {"text": "bar", "confidence": "99"}, similarity will be 0 and it won't log anything

Because I only found it useful

I don't see the point in having it exactly the same. If you are looking for "fooo" and OCR gets "foo0", you may not want to match it, because the similarity is 75, but it's close enough to your threshold that you may want it to be logged.
If you have the exact same value, you are basically filtering for similarity to see when the confidence is too low but the similarity is high.

I think having a log threshold right below your real threshold will be useful to log only the cases that are close, so you can adjust your thresholds accordingly.

[adombeck]

But in that case you won't log anything. Both have to be close to the threshold to log anything. Imagine you text is blurry, but the OCR still manages to get your word right. You try to look for foo, and you get: {"text": "foo", "confidence": "65"}. The OCR was close to get it, but the confidence was just below the threshold. I think that information could be useful.

You already get that message with my proposal though, because the similarity between "foo" and "foo" is 100, so it's above the similarity log threshold. In the diff you suggested, you just add an additional condition for logging the rejected matches, i.e. that the confidence is also high. That means when the text is blurry but the OCR engine still recognizes the text we're looking for, e.g. {"text": "foo", "confidence": "50"}, it will reject the match and won't even log about it.

I don't see the point in having it exactly the same. If you are looking for "fooo" and OCR gets "foo0", you may not want to match it, because the similarity is 75, but it's close enough to your threshold that you may want it to be logged. If you have the exact same value, you are basically filtering for similarity to see when the confidence is too low but the similarity is high.

That is a case we want to log IMO. In the case from above, when the engine is unsure that it recognized the exact text and returns {"text": "foo", "confidence": "50"}, we do want to log the rejected match even if the confidence score is very low.

I think having a log threshold right below your real threshold will be useful to log only the cases that are close, so you can adjust your thresholds accordingly.

Agreed, that's why in our tests we are using a similarity threshold of 92 and a log threshold of 80. My point is just that the default similarity threshold is already too low and causes too many false positives. I don't think we can safely change that, because it would break existing tests. So the proposed log threshold of 80 would mainly be useful when you set a higher similarity threshold (except for the case discussed above, where the match was rejected because of low confidence, in that case the log threshold of 80 would also be useful without setting a higher similarity threshold). Anyway, if we also make the log threshold configurable, which we should probably do anyway, I'm also fine with using a lower default there.

[fernando79513]

You already get that message with my proposal though.

You are 100% right there. Sorry for my confusion. The condition I suggested was even more restrictive.

If you think we don't need a lower bound to log the confidence results, I think that's okay. The lower bound for outputting a result (text_score) is already set at 0.5 in the rapidocr config.yaml, so I don't think it's going to introduce too much noise.

I didn't get at first why we would want to have a logging mechanism that just skips logging when using default values (for the similarity threshold). If you are only planning on increasing it, it makes sense because you will log the results you would get with the default values.
Nevertheless, if you were going to reduce the similarity threshold, you wouldn't log anything; that's why I liked the "LOG_THRESHOLD" approach.

Maybe the threshold is already too permissive, and we don't have to bother about it...

I leave it up to you if you want to have an absolute threshold for logs or a relative one.

[adombeck]

You are 100% right there. Sorry for my confusion.

No worries, I also found it challenging to understand the actual effect of the different approaches in practice and actually tried out the approaches to see the actual results.

If you are only planning on increasing it, it makes sense because you will log the results you would get with the default values.

Exactly, that was the idea.

Maybe the threshold is already too permissive, and we don't have to bother about it...

That's the case in my experience

[fernando79513]

LGTM +1

[adombeck]

rebased on main and signed my commits which were missing signatures

[adombeck]

Do you have a strict "only squash and merge" policy? I tried to make the commits self-contained and write useful commit messages, would be a shame to lose those.

[p-gentili]

Do you have a strict "only squash and merge" policy? I tried to make the commits self-contained and write useful commit messages, would be a shame to lose those.

We do, because versioning is computed automatically, but I can make it work for you. I just need you to rename all those commits removing any sort of prefix.

[adombeck]

I can make it work for you.

Thank you!

I just need you to rename all those commits removing any sort of prefix.

Done!