Maratyszcza / NNPACK

Acceleration package for neural networks on multi-core CPUs
BSD 2-Clause "Simplified" License
1.68k stars 316 forks source link

Bug? #110

Closed ghost closed 7 years ago

ghost commented 7 years ago

In the file /src/ref/fft/forward-dualreal.c at line 234 and 235

f[32] = x8; // bug? this line asigns a complex value to a float value ? f[33] = h8; // bug? this line asigns a complex value to a float value ?

Use x16 ans h16 instead ?

Did a fresh install of NNPACK ninja smoketest runs without an error. ninja test stops early with a floating point exception. Is this ok?

Thanks

Maratyszcza commented 7 years ago

Right, lines 234-235 should assign x16/h16. Good catch! However, this function is not used because I never did an optimized version of FFT32.

Running ninja test on a fresh NNPACK checkout now.

ghost commented 7 years ago

I'm running Ubuntu 17.10 on a virtual machine under Windows 10 Hyper-V Could that be the culprit?

Maratyszcza commented 7 years ago

I coudn't reproduce the crash on Mac with Apple LLVM version 8.1.0 (clang-802.0.42). What is your gcc version on Ubuntu?

ghost commented 7 years ago

After installing the latest llvm 5.0 and do fresh build ninja test runs just fine now.

I have a strange failure in the fourier unit test (FFT16_WITHIN_ROWS.match_reference) om my windows port. The function fft16_within_rows in complex_soa.py is doing well on Linux but gives an error on Windows. Is PeachPy producing a faulty function in the object file here?

thanks

ghost commented 7 years ago

Disabled some compiler optimizations and now the FFT16_WITHIN_ROWS.match_reference unit test run without issues now. I still have some other bugs to squash in the convolution part in nnpack-windows.
Now I'm at least sure there are no lingering bugs in your code!

Thanks