Skip to content

Conversation

@jnsiemer
Copy link
Member

@jnsiemer jnsiemer commented Nov 20, 2025

Description

This PR does the following:

  • ensure that its implementation is up-to-date with the current version of math and uses the correct version
  • adds lossy compression as defined for ML-KEM with implementations for PolynomialRingZq and MatPolynomialRingZq.

@jnsiemer jnsiemer self-assigned this Nov 20, 2025
@jnsiemer jnsiemer marked this pull request as ready for review December 9, 2025 14:54
Copy link
Member

@Marvin-Beckmann Marvin-Beckmann left a comment

Choose a reason for hiding this comment

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

I would make the following suggestions:

  1. make the output of the compression have the different explicit modulus as described in the FIPS document. Therefore I suggest to either: Make a new modulus object, that you potentially provide, or: make the output type only capture the values of the different coefficients/coordinates of coefficients. (Otherwise inintended behavior can easily occur if the compressed values are used beyond just compressing and decompressing
  2. I think individual functions rather than a trait is a bit better here personally. Especially if you implement the previous suggestions, a trait does not necessarily make too much sense anymore. Also: the traitname suggests that by implementing it like this: this lossycompression is the only possible lossycompression technique there is. If you make the name more tailored to the actual functionality, then you can also add individual function names, which is what I would opt for here.

}
}

impl LossyCompression for MatPolynomialRingZq {
Copy link
Member

Choose a reason for hiding this comment

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

same comments

@jnsiemer jnsiemer changed the title Update Update + Lossy Compression Dec 10, 2025
@jnsiemer jnsiemer merged commit d9d4a9c into dev Dec 12, 2025
2 checks passed
@jnsiemer jnsiemer deleted the update branch December 12, 2025 11:02
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