This is designed to fix issue #140 and prevent similar issues in the future by having checks run on new PRs.
It turns out there was more than one thing wrong with the builds, so the list of fixes is:
Issue #140 caused by a std::min() call in bitpacking_io.cpp
tile_matrix(mode="fragments") had a broken SIMD implementation that was only caught with 4-wide SIMD
sctransform_pearson() had lower precision than it should have on ARM due to a typo when calculating Newton-Raphson iterations of ReciprocalSqrt for iterations >1
Don't use an unoptimized -O0 build for windows, as there is a strange compiler bug that gets triggered affecting SIMD code and causing segfaults.
In short, the crash is from performing an aligned load of an AVX vector argument from an unaligned stack address. (I confirmed this is happening in gdb). AVX requires 32-byte alignment, but the stack only guarantees 16 I believe. Optimized builds cause all these functions to be inlined so there is no issue reading vector arguments from the stack.
There remain two unfixed issues to follow up on. For now, the relevant tests have been marked to skip:
Code related to peak calling uses parallel::mclapply() for parallelism which isn't supported on windows.
Moving this parallelism into C++ would fix the problem.
The calls to macs subprocesses might be tricky, particularly while having Ctrl-C get handled properly. the processx R library may have some useful learnings to supply (e.g. this blog post)
Maximum open file issues happen for me on the windows CI
This issue doesn't reproduce for me testing locally on my windows machine
This should be handled by _setmaxstdio calls in binaryfile.h, but maybe we can do some print debugging to see if those calls are failing in the CI environment.
A future follow-up goal to improve CI would be fixing all the skipped tests and adding test coverage tracking via covr
This is designed to fix issue #140 and prevent similar issues in the future by having checks run on new PRs.
It turns out there was more than one thing wrong with the builds, so the list of fixes is:
std::min()
call inbitpacking_io.cpp
tile_matrix(mode="fragments")
had a broken SIMD implementation that was only caught with 4-wide SIMDsctransform_pearson()
had lower precision than it should have on ARM due to a typo when calculating Newton-Raphson iterations ofReciprocalSqrt
for iterations >1-O0
build for windows, as there is a strange compiler bug that gets triggered affecting SIMD code and causing segfaults.There remain two unfixed issues to follow up on. For now, the relevant tests have been marked to skip:
parallel::mclapply()
for parallelism which isn't supported on windows.macs
subprocesses might be tricky, particularly while having Ctrl-C get handled properly. theprocessx
R library may have some useful learnings to supply (e.g. this blog post)_setmaxstdio
calls inbinaryfile.h
, but maybe we can do some print debugging to see if those calls are failing in the CI environment.A future follow-up goal to improve CI would be fixing all the skipped tests and adding test coverage tracking via
covr