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

Al/refactor lut #2001

Merged
merged 1 commit into from
Jan 27, 2025
Merged

Al/refactor lut #2001

merged 1 commit into from
Jan 27, 2025

Conversation

agnesLeroy
Copy link
Contributor

@agnesLeroy agnesLeroy commented Jan 24, 2025

closes: please link all relevant issues

PR content/description

This PR introduces degree / max degree data in the int_radix_lut structure, to prepare for noise tracking in apply_uni/bivariate_lookup_table.

I chose to track max_degree as well because for the many lut case it depends on the number of luts applied.

Check-list:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • Relevant issues are marked as resolved/closed, related issues are linked in the description
  • Check for breaking changes (including serialization changes) and add them to commit message following the conventional commit specification

@cla-bot cla-bot bot added the cla-signed label Jan 24, 2025
@agnesLeroy agnesLeroy marked this pull request as ready for review January 24, 2025 14:45
@agnesLeroy agnesLeroy requested review from bbarbakadze, pdroalves and guillermo-oyarzun and removed request for bbarbakadze January 24, 2025 14:45
@agnesLeroy agnesLeroy force-pushed the al/refactor_lut branch 2 times, most recently from c5896c6 to bc5e468 Compare January 24, 2025 15:44
@guillermo-oyarzun
Copy link
Member

Maybe it would be good to launch the long tests on this PR before merging it, mainly to test the many combinations of the luts

@agnesLeroy
Copy link
Contributor Author

I'll do a quick run locally with less ops. Since only the C++ side is modified, this does not affect the computation of noise level/degree on the Rust side yet. Next PR should do that.

@guillermo-oyarzun
Copy link
Member

I'll do a quick run locally with less ops. Since only the C++ side is modified, this does not affect the computation of noise level/degree on the Rust side yet. Next PR should do that.

ahh ok, then maybe it is not needed, if nothing is using this yet, probably we won't see anything with the tests anyway

@agnesLeroy agnesLeroy merged commit d13336c into main Jan 27, 2025
146 checks passed
@agnesLeroy agnesLeroy deleted the al/refactor_lut branch January 27, 2025 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants