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

From_str for PolyRingZq #411

Merged
merged 11 commits into from
Sep 27, 2024
Merged

From_str for PolyRingZq #411

merged 11 commits into from
Sep 27, 2024

Conversation

Marcel583
Copy link
Member

@Marcel583 Marcel583 commented Sep 17, 2024

Description

This PR implements from_str for PolyRingZq and refactors Doc comments (casing and spacing in parameters, punctuation in Returns, Adding missing error explanations and minor spelling mistakes).

Testing

  • 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

@Marcel583 Marcel583 self-assigned this Sep 17, 2024
@Marcel583 Marcel583 added documentation📖 Improvements or additions to documentation enhancement📈 New feature labels Sep 17, 2024
@Marcel583 Marcel583 marked this pull request as ready for review September 17, 2024 16:29
@@ -31,7 +31,7 @@ impl MatZ {
/// to the standard deviation `sigma * sqrt(2 * pi) = s`
///
/// Returns a matrix with each entry sampled independently from the
/// specified discrete Gaussian distribution.
/// specified discrete Gaussian distribution or an error if the `n <= 1` or `s <= 0`.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// specified discrete Gaussian distribution or an error if the `n <= 1` or `s <= 0`.
/// specified discrete Gaussian distribution or an error if `n <= 1` or `s <= 0`.

@@ -81,7 +81,9 @@ impl MatZ {
/// - `s`: specifies the Gaussian parameter, which is proportional
/// to the standard deviation `sigma * sqrt(2 * pi) = s`
///
/// Returns a lattice vector sampled according to the discrete Gaussian distribution.
/// Returns a lattice vector sampled according to the discrete Gaussian distribution
/// or an error if the `n <= 1` or `s <= 0`, the number of rows of the `basis` and `center` differ,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// or an error if the `n <= 1` or `s <= 0`, the number of rows of the `basis` and `center` differ,
/// or an error if `n <= 1` or `s <= 0`, the number of rows of the `basis` and `center` differ,

see the others as well -> this occurs more often

Comment on lines 151 to +152
/// - Returns a [`MathError`] of type [`InvalidIntegerInput`](MathError::InvalidIntegerInput)
/// if the `base` is not greater than `1`.
/// if the `base` is smaller than `2`.
Copy link
Member

Choose a reason for hiding this comment

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

maybe look for subseuent changes in crypto

Comment on lines +184 to +186
mod test_from_str {
use super::PolynomialRingZq;
use std::str::FromStr;
Copy link
Member

Choose a reason for hiding this comment

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

you can also call to_string on PolynomialRingZq options. Please add a test where you instantiate a PolynomialRingZq, then call to_string and then use from_str to reinitiate

@Marcel583 Marcel583 merged commit ac1b49c into dev Sep 27, 2024
2 checks passed
@Marcel583 Marcel583 deleted the PolyRingZq_FromsStr branch September 27, 2024 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation📖 Improvements or additions to documentation enhancement📈 New feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants