<div dir="ltr"><div>Thanks very much James for the research and the patch.<br><br></div>I've sent it upstream: <a href="https://bitbucket.org/ignitionrobotics/ign-math/pull-requests/232/bug-in-pairing-function-insde-helperscc/diff">https://bitbucket.org/ignitionrobotics/ign-math/pull-requests/232/bug-in-pairing-function-insde-helperscc/diff</a><br></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jan 24, 2018 at 9:14 AM, James Cowgill <span dir="ltr"><<a href="mailto:jcowgill@debian.org" target="_blank">jcowgill@debian.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Control: tags -1 patch<br>
<br>
Hi,<br>
<div><div class="h5"><br>
On 23/01/18 23:48, Aaron M. Ucko wrote:<br>
> Source: ignition-math4<br>
> Version: 4.0.0+dfsg1-1<br>
> Severity: important<br>
> Tags: upstream<br>
> Justification: fails to build from source<br>
> User: <a href="mailto:debian-mips@lists.debian.org">debian-mips@lists.debian.org</a><br>
> Usertags: mips mipsel<br>
><br>
> Builds of ignition-math4 for mips, mipsel, and the non-release<br>
> architecture hppa have been failing as detailed at<br>
><br>
> <a href="https://buildd.debian.org/status/fetch.php?pkg=ignition-math4&arch=mips&ver=4.0.0%2Bdfsg1-1&stamp=1516376024&raw=0" rel="noreferrer" target="_blank">https://buildd.debian.org/<wbr>status/fetch.php?pkg=ignition-<wbr>math4&arch=mips&ver=4.0.0%<wbr>2Bdfsg1-1&stamp=1516376024&<wbr>raw=0</a><br>
> <a href="https://buildd.debian.org/status/fetch.php?pkg=ignition-math4&arch=mipsel&ver=4.0.0%2Bdfsg1-1&stamp=1516373688&raw=0" rel="noreferrer" target="_blank">https://buildd.debian.org/<wbr>status/fetch.php?pkg=ignition-<wbr>math4&arch=mipsel&ver=4.0.0%<wbr>2Bdfsg1-1&stamp=1516373688&<wbr>raw=0</a><br>
> <a href="https://buildd.debian.org/status/fetch.php?pkg=ignition-math4&arch=hppa&ver=4.0.0%2Bdfsg1-1&stamp=1516373442&raw=0" rel="noreferrer" target="_blank">https://buildd.debian.org/<wbr>status/fetch.php?pkg=ignition-<wbr>math4&arch=hppa&ver=4.0.0%<wbr>2Bdfsg1-1&stamp=1516373442&<wbr>raw=0</a><br>
><br>
> On mips, this test manages to make ctest itself to run out of memory:<br>
><br>
> Start 11: UNIT_Helpers_TEST<br>
> terminate called after throwing an instance of 'std::bad_alloc'<br>
> what(): std::bad_alloc<br>
> Makefile:143: recipe for target 'test' failed<br>
> make[2]: *** [test] Aborted<br>
><br>
> On mipsel and hppa, HelpersTest.Pair spews millions(!) of errors and<br>
> then hits a timeout:<br>
><br>
> 11/75 Test #11: UNIT_Helpers_TEST ......................***<wbr>Timeout 240.12 sec<br>
> [...]<br>
> [ RUN ] HelpersTest.Pair<br>
> /<<BUILDDIR>>/ignition-math4-<wbr>4.0.0+dfsg1/src/Helpers_TEST.<wbr>cc:516: Failure<br>
> Expected: a<br>
> Which is: 4294962295<br>
> To be equal to: c<br>
> Which is: 4294962296<br>
<br>
</div></div>This is caused by the pairing function in src/Helpers.cc<br>
<br>
Firstly, I am not sure I understand why anyone would need this pairing<br>
function exactly. The point of a pairing function is that the inputs can<br>
be infinitely big, but the inputs to these functions are 32-bit<br>
integers. As long as you don't need the specific algorithm, you could<br>
implement something similar using a single shift and an add.<br>
<br>
I think the issue is this code relies on infinite floating point<br>
precision in calculating the "sqrt" variable:<br>
> std::tuple<PairInput, PairInput> Unpair(const PairOutput _key)<br>
> {<br>
> // Must explicitly cast so that the _key is not auto cast to a double<br>
> uint64_t sqrt = static_cast<uint64_t>(<br>
> std::floor(std::sqrt(static_<wbr>cast<long double>(_key))));<br>
> uint64_t sq = sqrt * sqrt;<br>
><br>
> return ((_key - sq) >= sqrt) ?<br>
> std::make_tuple(static_cast<<wbr>PairInput>(sqrt),<br>
> static_cast<PairInput>(_key - sq - sqrt)) :<br>
> std::make_tuple(static_cast<<wbr>PairInput>(_key - sq),<br>
> static_cast<PairInput>(sqrt));<br>
> }<br>
<br>
On architectures with > 64-bit long doubles (like Linux x86_64) you get<br>
away with this. On other architectures where long double == double, the<br>
cast to long double and sqrt is too inaccurate to implement this properly.<br>
<br>
The attached patch is based on a more accurate integer square root I<br>
found on stack overflow. You should be able to drop your<br>
0004_test_failures_in_non_x64_<wbr>arches.patch after applying this and<br>
possibly some other #ifdefs I noticed that upstream have added.<br>
<span class="HOEnZb"><font color="#888888"><br>
James<br>
</font></span><br>--<br>
debian-science-maintainers mailing list<br>
<a href="mailto:debian-science-maintainers@lists.alioth.debian.org">debian-science-maintainers@<wbr>lists.alioth.debian.org</a><br>
<a href="http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/debian-science-maintainers" rel="noreferrer" target="_blank">http://lists.alioth.debian.<wbr>org/cgi-bin/mailman/listinfo/<wbr>debian-science-maintainers</a><br></blockquote></div><br></div>