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
Testsuite failures on selected architectures due to alignment problem #128
Comments
Ach, I've no idea why it's suddenly broken. The package is intended to be portable, but I've no mechanism for testing against anything but x86/x86-64. I know the alignment code for ARM is not optimised. What is the failure? |
You can click on individual
|
Ah, those are very specific tests for x86 like. I suspect it actually works just fine, but the tests should be modified to support alternative architectures - I knew this when I wrote them but didn't have the facility to implement them. I suggest this issue serves as a rallying point for attempts to solve the problem. PRs gratefully received. I'll get onto it at some point, but I'm pretty busy at the moment. |
FYI, |
@ghisvail Hey, sorry for the delay in getting back to you - I've been away for a couple of weeks. It seems rather extreme to drop to whole thing no? Surely there's some way to only support certain architectures? Is this fixed for the next release now? |
@ghisvail Any movement on this? I make no claims about supporting anything other than x86/x86-64, can this be reflected? |
I'll have to ask the release team to remove the packages from the failing architectures. At least I now have a public statement that upstream is not actively supporting anything other than i386 and x86_64, so that should justify it. |
Thank you. Also for the record, I intend to support ARM at some point, but it is not high priority at the moment and needs a little more thought. As you suggest, it might work currently but YMMV. Patches welcome. |
Here are the test failures currently on arm:
|
@ghisvail thanks. I don't think it's a big effort to fix those, so anybody that wants to try is welcome to do so. It really boils down to adding ARM specific bits where there are x86 specific bits. |
@hgomersall Just to let you know that |
Got here as I noticed my |
Well, I would have thought so, but apparently not. Does this mean all code that goes into Debian needs to support ARM? |
The rationales behind the removal are publicly listed here. |
These alignment test failures still occur in pyFFTW 0.11.1, see the red build logs at https://buildd.debian.org/status/package.php?p=pyfftw. What scope of work is needed to fix the arch-specific alignment code in pyFFTW? With regards to Debian support, we can deactivate specific arches if necessary, but the standard set of arches that Debian packages should normally support are amd64 arm64 armel armhf i386 mips mips64el mipsel ppc64el s390x. So normally support for ARM is expected. Support on any of the other arches (alpha, hppa, etc) is bonus. |
Can you try with PR #201? I didn't test it on ARM, so it has not been merged. If it does work, we can make a new release with the fix. |
The PR#201 patch helps in part, but some errors remain. On mipsel, test_auto_align_input fails on builders: but test_auto_align_input passes other interfaces. test_alignment now passes, but test_get_alignment gives an error (also on armel):
I'm attaching the mipsel build log. |
On armel the #201 patch passes all test_auto_align_input (including builders) and all test_alignment. test_get_alignment fails (rather than giving an "error" as on mipsel): FAIL: test_get_alignment (test.test_pyfftw_utils.UtilsTest) Build log attached |
I think there might be a bit of fiddling to get this working, though I suspect it isn't so hard - there is a bit of CPU inspection that goes on in the alignment code. What's the best hardware to test all this on? One really needs shell access to something to play around with the code. |
I've access to Debian armel and mipsel machines (and others) so I can proxy. I guess it's easier for you to fiddle directly, so I'll ask about access. |
Shouldn't be so hard to set up. https://dsa.debian.org/doc/guest-account/ I'll send you an email. |
x32 (amd64 with ILP32 instead of LP64 model) is also affected:
Unfortunately, the assertTrue makes it hard to test this; on 0.11.1-4 the first test seems to pass. You can test x32 on an amd64 machine in an x32 chroot, if you boot your amd64 kernel with the option |
Interesting… failures seem to depend on the system it’s built on (which means the package is dependent on the build system, which is a complete no-go for binary distributions). On my x32 box I get:
|
Okaaaaay… it gets better. When I ignore the test failure, let it create a
This is still in the clean/minimal build chroot. WTF‽ |
Weird... Is the alignment lookup code doing something odd? It probes the CPU and isn't very well tested (it's rather hard to test such super platform specific stuff). Are you able to play around with that a bit? |
I can install the built package and run a couple of test scripts, but I’m not very involved in Python and have no idea what fftw even is (just trying to keep the Debian/x32 port alive). I do know that alignment can be tricky: for example, an int is normally 4-byte aligned, but only 2-byte aligned on m68k, and some x86 SSE operations need 16-byte alignment. |
The lookup is done inside a c header, which is the potential problem point. It tries to detect capability (SSE or AVX) by probing the CPU registers and returns the correct alignment. That is exposed through |
@ghisvail What do we want to support? It would be good to extend to ARM, but do we want the other more obscure platforms? |
Debian build logs are collated at https://buildd.debian.org/status/package.php?p=pyfftw List of debian test machines is at https://db.debian.org/machines.cgi (look for porterbox), which has |
It also says the same for mips64el which is also failing. |
hmm, strange that the mips64el build didn't catch it as a failure |
Drew Parsons dixit:
Weird, that status page says x32 is fine, but the arch build log at
https://buildd.debian.org/status/logs.php?pkg=pyfftw&arch=x32 shows it
failing.
That’s because I uploaded the package manually with tests disabled.
Henry Gomersall dixit:
The lookup is done inside a [c
header](https://github.com/pyFFTW/pyFFTW/blob/master/include/cpu.h),
It does that only for x86. ARM is not x86 and so would not
trigger that code (statically return 4, which might not be
correct either). x32 *is* x86… just 64-bit x86 with 32-bit
pointers. (There’s an arm64ilp32 architecture in nascence,
which is the equivalent for arm64.)
bye,
//mirabilos
--
„Cool, /usr/share/doc/mksh/examples/uhr.gz ist ja ein Grund,
mksh auf jedem System zu installieren.“
-- XTaran auf der OpenRheinRuhr, ganz begeistert
(EN: “[…]uhr.gz is a reason to install mksh on every system.”)
|
Yeah right, so it (hopefully) just needs an equivalent bit of code for the other architectures. |
You can't support architectures you don't test against. However, you can ensure that architecture-sensitive code paths and tests are appropriately guarded so that builds on more exotic architectures don't accidentally fail when they should not or succeed when they should not have. |
When I tested the arm64, I had the same error. Is there a solution now? Does it support arm64?
When I tested the arm64, I had the same error. Is there a solution now? Does it support arm64? @hgomersall |
Not explicitly. If you want to fix it, it would be well received. I think it should be pretty simple. |
I have updated the Debian packaging with release 0.10.4 and the build now fails for some architectures, some of these considered major. On arm64, powerpc, ppc64el and s390x, the failing tests seem all related to alignment. If the package is intended to be not portable, please acknowledge it here and I'll restrict the build to the qualifying architectures.
Thanks,
Ghis
The text was updated successfully, but these errors were encountered: