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

float number compare is ambiguity in UPCEANReader.decodeDigit #1400

Closed
shirne opened this issue Jun 23, 2021 · 3 comments
Closed

float number compare is ambiguity in UPCEANReader.decodeDigit #1400

shirne opened this issue Jun 23, 2021 · 3 comments

Comments

@shirne
Copy link

shirne commented Jun 23, 2021

我在调试解码系统时发现一处浮点数比较不严谨的代码,可能导致歧义。调试界面截图如下:
When I was debugging the decoding system, I found a code with less rigorous floating-point numbers, which may cause ambiguity. The screenshot of the debugging interface is as follows:
aaa

此处循环前保留的最佳匹配度是 0.219780与当前计算的匹配度0.219780是完全一样的,但代码比较中使用了< ,并且通过了比较,导致bestMatch从1变成了2
The best matching degree retained before the loop here is 0.219780 and the current calculated matching degree 0.219780 is exactly the same, but the code comparison uses <and passes the comparison, resulting in the bestMatch changing from 1 to 2

如果此处相等的匹配度总是能通过,应该将 < 号修正为 <=,否则,应该使用指定精确度的比较
If the equal matching degree here always passes, the <sign should be corrected to <=, otherwise, the comparison with the specified accuracy should be used

重现代码如下:
The code to reproduce the bug is as follows:

Test code:

    Result result = null;
    Map<DecodeHintType,Object> hints = new EnumMap<>(DecodeHintType.class);

    hints.put(DecodeHintType.TRY_HARDER, Boolean.TRUE);

    int[] bits = {0, 0, 0, 0, 0, 0, 0, 0, -33570816, 118, 0, 0, 946064924, -2026257522, 955253639, 124828, 0, 0, 0, 0};
    BitArray row = new BitArray(bits, 640);
    result = new UPCEReader().decodeRow(128, row, hints); //  here throws ChecksumException

此段代码在解码时得到的临时结果是 05221169 (如果浮点数比较指定了精度,得到的结果是 05111169 ,并且不会抛出 ChecksumException)
The temporary result of this code during decoding is 05221169 (if the precision is specified for the comparison of floating-point numbers, the result is 05111169 and ChecksumException will not be thrown)

在测试项目中新增一个测试方法,将以上代码复制过去,以Debug模式运行,运行前在 UPCEANReader.decodeDigit 的相关位置打好断点
Add a test method to the test project, copy the above code, run it in Debug mode, and set a breakpoint at the relevant position of UPCEANReader.decodeDigit before running

@srowen
Copy link
Contributor

srowen commented Jun 23, 2021

I don't understand why that's a bug in general - the new one is not better, they're judged to be the same.
It may well happen that one choice or the other works in one image and not another, but you'd probably fail as many images as you pass in these very narrow cases.

@shirne
Copy link
Author

shirne commented Jun 23, 2021

在逻辑上它确实不算bug,但是此段代码会产生歧义,并且在不同的语言上可能会导致不同的结果。
Logically, it is indeed not a bug, but this code will cause ambiguity and may lead to different results in different languages 。

我在尝试将这个库翻译为dart,在测试中发现这个问题导致 negative/partial_black_box_test 中的 11.png 无法通过测试。所以来反馈一下,看看是否能更完美
I'm trying to translate this library to dart, and found that this problem caused the 11.png in negative/partial_black_box_test to fail the test. So I give feedback and see if it can be more perfect

@srowen
Copy link
Contributor

srowen commented Jun 23, 2021

I don't think that's related to the comparison per se - maybe architecture or floating point semantics in dart? it just happens to be equal and happens to be the difference here. Floating point equality and comparisons are entirely well-defined and unambiguous. I think this change would equally cause some other image to fail, just not one in the test set. In any event, this project is Java only but you can modify a port or test cases as you see fit.

@srowen srowen closed this as completed Jun 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants