Skip to content

NTT Implementation for PolynomialRingZq #462

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

Open
wants to merge 34 commits into
base: dev
Choose a base branch
from

Conversation

Marvin-Beckmann
Copy link
Member

@Marvin-Beckmann Marvin-Beckmann commented Feb 25, 2025

Description

This PR implements the NTT for

  • Cyclic Modulus
  • Negacyclic Modulus
  • multiplication of PolynomialRingZq

Even though I included several optimisations I found, I did not manage to make the implication faster than direct multiplication using FLINT, where the multiplication is already very fast.
I included benchmarks and added an additional disclaimer in the comment of the NTT-multiplication that is is not slower.
I tested it for common parameter-sets that I have found.
I found that the overhead necessary for multithreading also exceeds the runtime of the multiplication, so this was not an option to reduce the runtime, but for future potential optimisations I kept the framework to enable multi-threading within the code-base.

Testing

  • I added basic working examples (possibly in doc-comment)
  • 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 and others added 30 commits February 5, 2025 15:32
@Marvin-Beckmann Marvin-Beckmann added enhancement📈 New feature performance⚡ This issue or PR should improve labels Feb 25, 2025
@Marvin-Beckmann Marvin-Beckmann self-assigned this Feb 25, 2025
@Marvin-Beckmann Marvin-Beckmann requested review from LPorzenheim and removed request for jnsiemer July 2, 2025 09:57
/// This function allows to initialize a [`NTTBasisPolynomialRingZq`]
/// object.
/// We currently only allow for two kinds of moduli to accompany the construction:
/// It must be either cyclotomic (`X^n - 1`) or negacyclic (`X^n + 1`) convoluted wrapping (also submitted in the algorithm)
Copy link
Member

Choose a reason for hiding this comment

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

cyclic

/// Parameters:
/// - `n`: the degree of the polynomial
/// - `root_of_unity`: the `n`-th or `2n`-th root of unity
/// - `q`: the modulus of the cyclotomic polynomial
Copy link
Member

Choose a reason for hiding this comment

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

Misleading to call it cyclotomic because it only applies to one case?

/// This function essentially computes the included butterliy computations for each provided
/// chunk.
/// The chunk is double the size of the stride.
/// The computation currently performs the standard butterly operation from Gentleman-Sande.
Copy link
Member

Choose a reason for hiding this comment

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

butterfly, see first sentence as well

}
res
}

Copy link
Member

Choose a reason for hiding this comment

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

Tests missing for intt working correctly as well as different modulus (as in ntt)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement📈 New feature performance⚡ This issue or PR should improve
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants