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

Matrix transpose bitorder #27

Merged
merged 3 commits into from
Aug 11, 2023
Merged

Matrix transpose bitorder #27

merged 3 commits into from
Aug 11, 2023

Conversation

sinui0
Copy link
Collaborator

@sinui0 sinui0 commented Jul 5, 2023

This PR flips the bit order of the resulting transposed matrix

Copy link
Member

@th4s th4s left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ack 👍

fn transpose_naive(data: &[u8], row_width: usize) -> Vec<u8> {
use itybity::*;

let bits: Vec<Vec<bool>> = data.chunks(row_width).map(|x| x.to_lsb0_vec()).collect();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't row_width the width in bits ? here it is treated as if it was in bytes

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is correct here to use the row width in bytes, because we iterate in chunks of byte rows and turn this chunk into bit rows. So in the beginning it is bytes and after this operation it is bits.

@themighty1 themighty1 self-requested a review July 13, 2023 07:27
Copy link
Collaborator

@themighty1 themighty1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some tests seem to be failing

Is there an easy way to test the "simd-transpose" feature from command line? I couldnt find one.
Maybe we should have our CI to also run the tests with "simd-transpose" enabled?

@sinui0
Copy link
Collaborator Author

sinui0 commented Aug 11, 2023

I don't see any failing tests, try cargo clean then cargo +nightly t --features simd-transpose

@sinui0
Copy link
Collaborator Author

sinui0 commented Aug 11, 2023

I'm going to merge this, it is blocking #37. Changes to CI etc can be done in another PR

@sinui0 sinui0 merged commit 6c4af5a into ot-work Aug 11, 2023
@sinui0 sinui0 deleted the bug/matrix-transpose-bitorder branch August 11, 2023 20:50
sinui0 added a commit that referenced this pull request Aug 30, 2023
* add naive test

* Change algorithm to work for LSB0 encoding instead of MSB0

* add test

---------

Co-authored-by: th4s <[email protected]>
Co-authored-by: themighty1 <[email protected]>
sinui0 added a commit that referenced this pull request Aug 30, 2023
* add naive test

* Change algorithm to work for LSB0 encoding instead of MSB0

* add test

---------

Co-authored-by: th4s <[email protected]>
Co-authored-by: themighty1 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants