Skip to content

Conversation

Biffo89
Copy link
Contributor

@Biffo89 Biffo89 commented Jul 16, 2025

The transpose for matrices over GF(2^e) is currently extremely slow due to the lack of mzed_transpose in M4RIE. I have made a PR to add one (malb/m4rie#8), but in the mean time, this is a temporary performance fix direct in Sage.

Currently, the transpose is costly enough that for medium-sized matrices it takes a significant amount of the time of pivot_rows():

sage: fields = [GF(2**2),GF(2**4),GF(2**8),GF(2**16),GF(next_prime(2**20))]
sage: for field in fields:
....: M = matrix.random(field,512,512)
....: print(field)
....: timeit('M._clear_cache();M.transpose()')
....: timeit('M._clear_cache();M.pivot_rows()')
....:
Finite Field in z2 of size 2^2
5 loops, best of 3: 128 ms per loop
5 loops, best of 3: 145 ms per loop
Finite Field in z4 of size 2^4
5 loops, best of 3: 130 ms per loop
5 loops, best of 3: 139 ms per loop
Finite Field in z8 of size 2^8
5 loops, best of 3: 145 ms per loop
5 loops, best of 3: 155 ms per loop
Finite Field in z16 of size 2^16
5 loops, best of 3: 1.12 s per loop
5 loops, best of 3: 3.82 s per loop
Finite Field of size 1048583
625 loops, best of 3: 1.15 ms per loop
25 loops, best of 3: 33.6 ms per loop

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation and checked the documentation preview.

⌛ Dependencies

@Biffo89
Copy link
Contributor Author

Biffo89 commented Jul 16, 2025

With change, transpose times are a lot less.

sage: fields = [GF(2**2),GF(2**4),GF(2**8),GF(2**16),GF(next_prime(2**20))]
sage: for field in fields:
....: M = matrix.random(field,512,512)
....: print(field)
....: timeit('M._clear_cache();M.transpose()')
....: timeit('M._clear_cache();M.pivot_rows()')
....:
Finite Field in z2 of size 2^2
125 loops, best of 3: 2.09 ms per loop
125 loops, best of 3: 4.88 ms per loop
Finite Field in z4 of size 2^4
125 loops, best of 3: 3.89 ms per loop
25 loops, best of 3: 10.1 ms per loop
Finite Field in z8 of size 2^8
125 loops, best of 3: 4.51 ms per loop
25 loops, best of 3: 25 ms per loop
Finite Field in z16 of size 2^16
125 loops, best of 3: 6.16 ms per loop
5 loops, best of 3: 2.72 s per loop
Finite Field of size 1048583
625 loops, best of 3: 1.21 ms per loop
25 loops, best of 3: 34 ms per loop

@xcaruso xcaruso added the gsoc: 2025 Tag for GSoC2025 issues/PRs label Jul 17, 2025
@vneiger
Copy link
Contributor

vneiger commented Jul 19, 2025

Just a minor note: please think about using markers to make code more readable in text for PR / Issues (typically multiline code will be enclosed with a line containing ``` before and the same one after).

@vneiger
Copy link
Contributor

vneiger commented Jul 19, 2025

Code looks good to me, just two minor suggestions:

For this second item, this is notably to make the examples more robust and avoid assuming Sage will always choose z2 for the generator.

@xcaruso
Copy link
Contributor

xcaruso commented Jul 21, 2025

Also, I think that the same comment as in #40435 applies.

Biffo89 and others added 3 commits July 21, 2025 17:20
Co-authored-by: Xavier Caruso <xcaruso@users.noreply.github.com>
Co-authored-by: Xavier Caruso <xcaruso@users.noreply.github.com>
Co-authored-by: Xavier Caruso <xcaruso@users.noreply.github.com>
@Biffo89
Copy link
Contributor Author

Biffo89 commented Jul 21, 2025

Also, I think that the same comment as in #40435 applies.

I just have two comments:

in the generic implementation, you can use get_unsafe and set_unsafe,

in the doctest of copy_from_unsafe, you should insist on the fact that the method assumes that src has also type MatrixGF2e_dense.

get_unsafe and set_unsafe introduce unnecessary conversion into a field element. As the two matrices are over the same field, it is faster to pass the raw return value of mzed_read.

@xcaruso
Copy link
Contributor

xcaruso commented Jul 21, 2025

Yes, I agree. I was talking about the generic implementation.

@xcaruso
Copy link
Contributor

xcaruso commented Jul 24, 2025

After #40435, do you want to close this PR?

@Biffo89
Copy link
Contributor Author

Biffo89 commented Jul 24, 2025

After #40435, do you want to close this PR?

Yes, after #40435 this PR is no longer needed.

@xcaruso
Copy link
Contributor

xcaruso commented Jul 25, 2025

Covered by #40435

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gsoc: 2025 Tag for GSoC2025 issues/PRs r: duplicate r: wontfix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants