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

Runtime tests are failing on several hardware architectures #720

Open
tillea opened this issue Dec 11, 2017 · 12 comments
Open

Runtime tests are failing on several hardware architectures #720

tillea opened this issue Dec 11, 2017 · 12 comments

Comments

@tillea
Copy link

tillea commented Dec 11, 2017

Hi,
the Debian package of bcftools received a bug report pointing to lots of build time test failures for several architectures. For instance you can check a log of a build on i386 (just seek for the string ".. failed ..."). There is an overview page about all architectures which leads to the conclusion that there are different types of failures for different architectures (wild guessing from different numbers of failures per some architectures while other architectures show the same numver failures).
While most probably bcftools is mostly run on amd64 architecture and also arm it is comiling fine this kind of errors typically are hints for issues in the code that deserves fixing in general.
Please note that while the bug report is against version 1.3.1 the logs are belonging also to version 1.5.4.
Kind regards, Andreas.

@jkbonfield
Copy link
Contributor

When I was doing the windows port a huge number of bcftools failures were down to variation in the last digit of a floating point number - ie rounding differences. I haven't looked at this particular issue, but we really need a more robust comparison mechanism to determine equality.

The other thing we need to be careful of is assuming zlib. There are many equally valid deflate streams, but IIRC using say the intel deflate causes the test harness to fail too.

@tillea
Copy link
Author

tillea commented Dec 11, 2017 via email

@pd3
Copy link
Member

pd3 commented Dec 11, 2017

A simple and quick hack for now is to modify test.pl to round float values first before comparing.

@tillea
Copy link
Author

tillea commented Dec 11, 2017 via email

@pd3
Copy link
Member

pd3 commented Dec 12, 2017

Thinking about it more, this is actually the right place to do it. The program works correctly, these rounding problems are just an artifact of evaluation. The rounding would be done on the strings entering the comparison here https://github.com/pd3/bcftools/blob/develop/test/test.pl#L426

@jkbonfield
Copy link
Contributor

Agreed @tillea this is something for us to do. There may be code already for handling this for Windows; eg lines 450ish in https://github.com/samtools/bcftools/pull/609/files?diff=split#diff-5e25dd32ec9d21cde3301825bf56195c

That was mainly for how exponents are printed, but it would be easy enough to do a regexp matching exponentials and reformat using a limited precision printf command.

I'm not sure ALL of the errors will be this nature though. I suspect there are other things which are failures-in-waiting, such as relying on zlib output to always match (it may not if we used another zlib library). We need to build a virtual machine somewhere I guess to test these whacky platforms.

@tillea
Copy link
Author

tillea commented Dec 13, 2017

Just for the sake of completeness: I've built bcftools version 1.6 for Debian experimental and the issue has not changed in the latest version.

pd3 added a commit that referenced this issue Dec 13, 2017
Most of these are cosmetic, but some are likely to fix at least some of the
platform-dependant test failures reported in #720. Specifically the merging
code relies on the correct number of fields in the FORMAT/Number=G tags.
@pd3
Copy link
Member

pd3 commented Dec 13, 2017

Can you try with this latest commit please? I expect some of the failures might be fixed by this. For the rest, it would be helpful to see all the /<<PKGBUILDDIR>>/test/*.new files created by the test script.
Thank you

@tillea
Copy link
Author

tillea commented Dec 14, 2017 via email

@mr-c
Copy link

mr-c commented Feb 20, 2020

2020 update: tests still fail on s390x, a big-endian architecture

samtools/htslib#1023 (comment)

https://buildd.debian.org/status/fetch.php?pkg=bcftools&arch=s390x&ver=1.10.2-2&stamp=1581632647&raw=0

Number of tests:
total .. 961
passed .. 936
failed .. 25

One example, as noted in samtools/htslib#355 (comment)

test_naive_concat:
	/<<PKGBUILDDIR>>/bcftools concat --naive-force /tmp/kUdZ7xZSaN/naive_concat.0.bcf /tmp/kUdZ7xZSaN/naive_concat.1.bcf /tmp/kUdZ7xZSaN/naive_concat.2.bcf /tmp/kUdZ7xZSaN/naive_concat.3.bcf /tmp/kUdZ7xZSaN/naive_concat.4.bcf /tmp/kUdZ7xZSaN/naive_concat.5.bcf /tmp/kUdZ7xZSaN/naive_concat.6.bcf /tmp/kUdZ7xZSaN/naive_concat.7.bcf /tmp/kUdZ7xZSaN/naive_concat.8.bcf /tmp/kUdZ7xZSaN/naive_concat.9.bcf | /<<PKGBUILDDIR>>/bcftools view -H

	Non-zero status 255

		Concatenating /tmp/kUdZ7xZSaN/naive_concat.0.bcfFailed to read from standard input: unknown file type
		

.. failed ...

@tillea
Copy link
Author

tillea commented Dec 17, 2020

End of 2020 update: tests still fail on big-endian architectures like s390x, ppc64 and others which you can chech here.

@mr-c
Copy link

mr-c commented Mar 5, 2024

Down to 12 failed tests with bcftools 1.19 on s390x: https://buildd.debian.org/status/fetch.php?pkg=bcftools&arch=s390x&ver=1.19-1&stamp=1702672304&raw=0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants