Skip to content

Conversation

@3pmile
Copy link
Member

@3pmile 3pmile commented Jul 18, 2025

Description

This PR implements MulAssign for scalars of

  • MatZ: signed and unsigned integers, Z
  • PolyOverZ: signed and unsigned integers, Z
  • MatPolyOverZ: signed and unsigned integers, Z, PolyOverZ
  • PolyOverZq: signed and unsigned integers, Z, Zq
  • PolynomialRingZq: signed and unsigned integers, Z, Zq
  • MatPolynomialRingZq: signed and unsigned integers, Z, Zq, PolyOverZq, PolynomialRingZq
  • PolyOverQ: signed and unsigned integers, Z, Q, f32, f64
  • MatQ: signed and unsigned integers, Z, Q, f32, f64

It also implements the following DivAssign (and small fixes in Div)

  • MatQ (Div + DivAssign): signed and unsigned integers, Z, Q, f32, f64
  • PolyOverQ (DivAssign): signed and unsigned integers, Z, Q, f32, f64

Testing

I decided against adding a complete test coverage for all combinations of borrowed and owned, and I did not manually check all div by 0 errors that I could find, as that is not needed, and also covered in flint itself (it returns an error from there, just not a classical panic from rust, but the program is terminated regardless).

  • I added basic working examples (possibly in doc-comment)
  • I added tests for large (pointer representation) values
  • I triggered all possible errors in my test in every possible way
  • I included tests for all reasonable edge cases

Checklist:

  • I have performed a self-review of my own code
    • The code provides good readability and maintainability s.t. it fulfills best practices like talking code, modularity, ...
      • The chosen implementation is not more complex than it has to be
    • My code should work as intended and no side effects occur (e.g. memory leaks)
    • The doc comments fit our style guide
    • I have credited related sources if needed

@Marvin-Beckmann Marvin-Beckmann self-assigned this Oct 8, 2025
@Marvin-Beckmann Marvin-Beckmann added bug🐛 Something isn't working enhancement📈 New feature labels Oct 8, 2025
Copy link
Member

@jnsiemer jnsiemer left a comment

Choose a reason for hiding this comment

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

Nice work!
Found several small issues (almost nothing functional, mostly documentation consistency). Please take the time to check all marked bugs yourself across the documents. I've spotted some comparatively late in the review.

@Marvin-Beckmann Marvin-Beckmann merged commit 5f50c9c into dev Oct 16, 2025
2 checks passed
@Marvin-Beckmann Marvin-Beckmann deleted the scalar_mulAssign branch October 16, 2025 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug🐛 Something isn't working enhancement📈 New feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants