-
-
Notifications
You must be signed in to change notification settings - Fork 11
CI: Adding and testing Big-Endian support for quaddtype #152
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
Conversation
I am guessing there might be more failures, eg: str and repr methods of dtype that uses dragon4, we can extensively test them later or maybe in this PR, whatever sounds good |
Self note: But the main purpose of this PR is CI and that is working |
@ngoldbaum this is ready now, let me know your review |
Thanks @SwayamInSync for your work on this! |
#if defined(__BYTE_ORDER__) && (__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__) | ||
npy_uint64 hi; | ||
npy_uint64 lo; | ||
#else |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! glad you chased this down. While BigEndian machines are rare these days it's nice to have fully correct code and it's important for it to be reflected in the source code that this representation is endian-dependent.
path: ~/docker_${{ matrix.BUILD_PROP[1] }} | ||
key: container-quaddtype-${{ runner.os }}-${{ matrix.BUILD_PROP[1] }}-${{ matrix.BUILD_PROP[2] }}-${{ hashFiles('quaddtype/pyproject.toml') }} | ||
|
||
- name: Create cross-compilation container |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This step takes 10 minutes. It might be possible to add some caching to make that quicker. Just a note for a future improvement.
Thanks @SwayamInSync! |
closes #119
I took inspiration from NumPy's workflow for this https://github.com/numpy/numpy/blob/main/.github/workflows/linux_qemu.yml
Since little-endian are already available in our current CI so here only big-endian testing.
Things seem to work and one test case is failing. Making the PR because CI is seems to work as expected