Re: [PATCH] d3dx9/tests: Provide 8 significant digits in matrix reference values

[Henri Verbeet]

Thanks for fixing these.

2017-11-09 6:10 GMT+01:00 Alex Henrie :
> Signed-off-by: Alex Henrie
>
> Fixes test failures on 64-bit Linux.
>
> A 32-bit float can hold 7.2 decimal digits, so it's best to provide 8 to
> make sure that the correct values are accurately represented.

If it were up to me, I'd prefer these to be in the same %.8e format
the failure messages use.
> The correct outputs of functions that take simple floats as inputs were
> computed by feeding equations into Wolfram Alpha.

I think that's probably fine, but note that we don't care about the
"correct" values so much as "whatever Windows produces". Hopefully
those would be reasonably close, but they're by no means guaranteed to
be the same.

[PATCH] d3dx9/tests: Provide 8 significant digits in matrix reference values

[Alex Henrie]

2017-11-09 1:52 GMT-07:00 Henri Verbeet :
> Thanks for fixing these.

You're welcome!
> 2017-11-09 6:10 GMT+01:00 Alex Henrie :
> > Signed-off-by: Alex Henrie
> >
> > Fixes test failures on 64-bit Linux.
> >
> > A 32-bit float can hold 7.2 decimal digits, so it's best to provide 8 to
> > make sure that the correct values are accurately represented.
>
> If it were up to me, I'd prefer these to be in the same %.8e format
> the failure messages use.

OK. Let's wait and see what Matteo wants.
> > The correct outputs of functions that take simple floats as inputs were
> > computed by feeding equations into Wolfram Alpha.
>
> I think that's probably fine, but note that we don't care about the
> "correct" values so much as "whatever Windows produces". Hopefully
> those would be reasonably close, but they're by no means guaranteed to
> be the same.

That's a good point, but unless there's an application that depends on
the specific rounding error from Windows, the exact values will be
fine. Using exact values will also keep the tests passing if the
rounding error changes between Windows versions or CPU architectures.

-Alex

[PATCH] d3dx9/tests: Provide 8 significant digits in matrix reference values

[Matteo Bruni]

2017-11-09 13:16 GMT+01:00 Alex Henrie :
> 2017-11-09 1:52 GMT-07:00 Henri Verbeet :
> > Thanks for fixing these.
>
> You're welcome!
> > 2017-11-09 6:10 GMT+01:00 Alex Henrie :
> > > Signed-off-by: Alex Henrie
> > >
> > > Fixes test failures on 64-bit Linux.
> > >
> > > A 32-bit float can hold 7.2 decimal digits, so it's best to provide 8 to
> > > make sure that the correct values are accurately represented.
> >
> > If it were up to me, I'd prefer these to be in the same %.8e format
> > the failure messages use.
>
> OK. Let's wait and see what Matteo wants.

In general IMO it would have been a bit nicer to just grab the results
from native x86 d3dx9 as output by the test (after e.g. replacing the
expected results with zeros or some other "wrong" value) and use them
as expected results. Also, since you're touching these lines anyway,
it could have been a good time to break those long lines to 100
columns or so and fix the whitespace in the tested calls (e.g.
D3DXMatrixLookAtLH())
I'm not going to hold the patch up for that though.
> > > The correct outputs of functions that take simple floats as inputs were
> > > computed by feeding equations into Wolfram Alpha.
> >
> > I think that's probably fine, but note that we don't care about the
> > "correct" values so much as "whatever Windows produces". Hopefully
> > those would be reasonably close, but they're by no means guaranteed to
> > be the same.
>
> That's a good point, but unless there's an application that depends on
> the specific rounding error from Windows, the exact values will be
> fine. Using exact values will also keep the tests passing if the
> rounding error changes between Windows versions or CPU architectures.

I don't think that's all that valuable in practice, at least for
d3dx9. In general we want to check for and reproduce Windows results
(and specifically, x86 results, which is 99.99% of the use cases for
d3dx9). Also the d3dx9 DLLs are "special" in that whenever they got
updated in a newer DirectX SDK version, they got a separate
d3dx9._xx.dll, leaving the old version around. Which also means that
applications might (thus, of course, do) actually depend on
implementation details like this one.

In this case, at least looking at the first test you updated in this
patch, it looks like the linux gcc-generated implementation is more
accurate than the "native" Windows one, for both x86 and x64. I don't
think we care enough about the small discrepancy to look in detail
into it.