Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

t/31.schema.t fails tests 238 and 3838 when nvtype is IBM DoubleDouble #33

Closed
sisyphus opened this issue Jun 25, 2020 · 4 comments
Closed

Comments

@sisyphus
Copy link

Hi,

This is a -Duselongdouble build of perl-5.32.0 (and 5.31.0 suffers the same problem) on Debian wheezy:

$ perl -V:longdblkind
longdblkind='6';

The errors are reported in the test suite as follows:

t/31.schema.t .............. 20/?
# Failed test '(yaml11) type float: load(!!float 190:20:30.15) eq '685230.15''
# at t/31.schema.t line 138.
# got: 685230.15
# expected: 685230.15
t/31.schema.t .............. 3615/?
# Failed test '(yaml11) type float: load(190:20:30.15) eq '685230.15''
# at t/31.schema.t line 138.
# got: 685230.15
# expected: 685230.15
t/31.schema.t .............. 4123/? # Looks like you failed 2 tests of 4394.
t/31.schema.t .............. Dubious, test returned 2 (wstat 512, 0x200)
Failed 2/4394 subtests

and, at the end of the test suite:

t/31.schema.t (Wstat: 512 Tests: 4394 Failed: 2)
Failed tests: 238, 3838
Non-zero exit status: 2

The problem is that $data != $def{data} in this particular case, though they are both (obviously) stringifying to the same string of "685230.15".
For this particular case
scalar(reverse(unpack "h*", pack("D<", $data))) is 4124e95c4ccccccdbdb999999999999a
scalar(reverse(unpack "h*", pack("D<", $def{data}))) is 4124e95c4ccccccdbdb9999999999998
which is a very minor discrepancy (of 2 units in the last place).

The former value (ie $data) is the correct representation of 685230.15 for this architecture, though both perl and C will inaccurately assign the latter value (which translates to 685230.150000000000000000000000005.
(The least significant double is a negative value, so $def{data} is in fact greater than $data, despite the appearance to the contrary.)

Could the test be changed from "==" to "eq" ? (The diagnostic message suggests that the intention was to test "eq", not "==".)

Cheers,
Rob

perlpunk added a commit that referenced this issue Jun 29, 2020
@perlpunk
Copy link
Owner

perlpunk commented Jun 29, 2020

Thanks!

I actually want to compare via == and not eq ($def{data} could contain 300.00 for example).
I just didn't think of floating point arithmetic problems, although I should have known it ;-)

I fixed the test simply by avoiding rounding errors with sprintf in 9e907d1

Could you test it?

@sisyphus
Copy link
Author

It seems that the first change where cmp_ok() is changed from performing an 'eq' test to an '==' might be the wrong thing.
It produces thousands of non-numeric warnings of the form:
Argument "*" isn't numeric in numeric eq (==) at (eval in cmp_ok) t/31.schema.t line 120.
where "*" is such things as "true", "y", "yes", "no", "null", "off", "on", "~", to name a few.

That's not causing any tests to fail, but for me the second change, where $data is replaced by sprintf("%0.2f",$data), is alone sufficient to allow all tests to pass.

However, I don't think I'm using the exact same version of 31.schema.t as is presented at 9e907d1 .
What seems to be line 128 of that file is actually line 120 on my copy of the file.

The interesting thing about that second change is that it fixes the failures by coercing the LHS of the '==' comparison to the same incorrect representation of 685230.15 as held by the RHS ($def{data}).
That approach is a little unusual and threw me initially - though it's a totally sane, valid and most practical solution.

Cheers,
Rob

perlpunk added a commit that referenced this issue Jul 3, 2020
See #33

Also use `header` option for dumper
@perlpunk
Copy link
Owner

perlpunk commented Jul 3, 2020

Huh, weird.
It looks like the change got accidentally applied to the $type eq 'str' condition on your side. Then I get the same warnings as you.
Here's the direct link to the correct file (branch testfloat):
https://github.com/perlpunk/YAML-PP-p5/blob/testfloat/t/31.schema.t

Thanks!

@sisyphus
Copy link
Author

sisyphus commented Jul 4, 2020

@sisyphus sisyphus closed this as completed Jul 4, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

2 participants