-
Notifications
You must be signed in to change notification settings - Fork 2
Feat/ggh lut 2601 #33
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
base: main
Are you sure you want to change the base?
Conversation
* add cuda * compile worked * GpuDCRTPoly tests worked * update build.rs * Add gpu_dcrt_poly matrix * add multi pipelines optimizations * fix compile errors * Revert "fix compile errors" This reverts commit d3bc55a. * Revert "add multi pipelines optimizations" This reverts commit 2ea09ec. * add tests * gpu matrix tests passed * Add gpu samplers * test_gpu_dcrtpoly~ and test_gpu_matrix~ tests passed * all gpu tests passed * use rayon more * cuda optimizations * Add test_arithmetic_circuit_operations_gpu_ggh15 * minor optimizations * optimize matrix mul and add benchmark codes * remove unused codes * fix clone of GpuDCRTPoly * optimize mul kernel * Add available_gpu_ids * CI: disable OpenMP in CMake configuration; remove memory logging for optimization * fix compile error * Feat/gpu without pipeline (#31) * Revert "fix compile errors" This reverts commit d3bc55a. * Revert "add multi pipelines optimizations" This reverts commit 2ea09ec. * add tests * gpu matrix tests passed * Add gpu samplers * test_gpu_dcrtpoly~ and test_gpu_matrix~ tests passed * all gpu tests passed * use rayon more * cuda optimizations * Add test_arithmetic_circuit_operations_gpu_ggh15 * minor optimizations * optimize matrix mul and add benchmark codes * remove unused codes * fix clone of GpuDCRTPoly * optimize mul kernel * Add available_gpu_ids * CI: disable OpenMP in CMake configuration; remove memory logging for optimization * fix compile error * make all tests sequential * fix compile errors
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f47dc8fd9d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Input is assumed to be a constant polynomial; use its constant coefficient as the key. | ||
| let const_coeff = input | ||
| .coeffs() | ||
| .first() | ||
| .cloned() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reject or handle non-constant LUT inputs
The new plaintext LUT evaluator derives the lookup key from only the first coefficient and always returns a constant polynomial. If any circuit applies public_lookup to a non-constant input (e.g., packed plaintext with multiple coefficients), the remaining coefficients are silently ignored and the lookup result becomes incorrect. This is a behavioral regression from the earlier “whole polynomial” lookup and will corrupt results unless all LUT inputs are guaranteed constant; consider validating constness or applying the LUT per coefficient instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We assume all LUT inputs are constant.
Can you add debug_assert lines to ensure that all inputs to the LUT is constant?
…r improved logging consistency
…chunk size, and modify the Clone impl of DCRTPoly
No description provided.