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

Feat: define objects over base fields #52

Open
wants to merge 25 commits into
base: feat/goldilocks-spartan
Choose a base branch
from

Conversation

kunxian-xia
Copy link
Collaborator

Fixes #50.

@darth-cy darth-cy self-assigned this Dec 4, 2024
@darth-cy darth-cy force-pushed the feat/goldilocks-spartan branch from 88029e3 to 5f86ada Compare December 5, 2024 07:10
@darth-cy darth-cy changed the base branch from feat/goldilocks-spartan to feat/remove_backend_zk December 9, 2024 23:21
@darth-cy darth-cy changed the base branch from feat/remove_backend_zk to feat/goldilocks-spartan December 10, 2024 01:18
@darth-cy darth-cy changed the base branch from feat/goldilocks-spartan to feat/remove_backend_zk December 10, 2024 01:19
Base automatically changed from feat/remove_backend_zk to feat/goldilocks-spartan December 10, 2024 01:23
Comment on lines 194 to 196
ops_addr: Vec<DensePolynomial<S, Ext>>,
read_ts: Vec<DensePolynomial<S, Ext>>,
audit_ts: DensePolynomial<S, Ext>,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These multilinear polynomials should be defined over Base.

Comment on lines 206 to 207
let mut ops_addr_vec: Vec<DensePolynomial<S, Ext>> = Vec::new();
let mut read_ts_vec: Vec<DensePolynomial<S, Ext>> = Vec::new();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

see above comments.

}
}

pub struct MultiSparseMatPolynomialAsDense<S: SpartanExtensionField> {
batch_size: usize,
val: Vec<DensePolynomial<S>>,
val: Vec<DensePolynomial<S, Ext>>,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This should be defined over Base. It's the M[i,j] where M \in {A, B, C}.

Comment on lines 259 to 260
comb_ops: DensePolynomial<S, Ext>,
comb_mem: DensePolynomial<S, Ext>,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These two multilinear polynomials should also be defined over Base.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

maximize the usage of S::BaseField to have smaller proof and better performance
2 participants