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

Add get lde functions #100

Merged
merged 55 commits into from
May 7, 2024
Merged

Add get lde functions #100

merged 55 commits into from
May 7, 2024

Conversation

sai-deng
Copy link

@sai-deng sai-deng commented May 2, 2024

I have to revert #91 to align the value domains.

@sai-deng sai-deng changed the title Sai/add get lde Add get lde functions May 4, 2024
@sai-deng sai-deng marked this pull request as ready for review May 4, 2024 03:34
@codeblooded1729
Copy link

codeblooded1729 commented May 7, 2024

I tried to work out why the mismatch was happening, and it seems that it was due to the following.
Basically the quotient polynomial constraint looks like
$$(x^n - 1) q(x) = VanishingPoly(x)$$
Due to the $x^n$ term, it becomes critical, over which coset of natural domain we would perfrom LDE (and commit) $q(x)$ on. That is we need to know the coset shift used (since $x^n$ evaluates to different values for different coset shift)
I see that current code performs LDE with constant coset shift given by F::coset_shift(), for all polynomials used. I am fine with this revert for now. Maybe in future we can try incorporating using custom coset shift into the logic used for quotient polynomials.

@sai-deng sai-deng merged commit 4a389fb into main May 7, 2024
6 checks passed
@sai-deng sai-deng deleted the sai/add_get_lde branch May 7, 2024 17:17
sai-deng added a commit that referenced this pull request Jul 18, 2024
I have to revert #91 to align the value domains.

---------

Co-authored-by: Kapil <[email protected]>
sai-deng added a commit that referenced this pull request Jul 18, 2024
I have to revert #91 to align the value domains.

---------

Co-authored-by: Kapil <[email protected]>
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.

2 participants