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

fix: rename q c field to constant in expression struct #7563

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions acvm-repo/acir/src/circuit/opcodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,12 @@ pub enum Opcode<F> {
/// `w=(w_1,..w_n)` is a tuple of `n` witnesses, and `P` is a multi-variate
/// polynomial of total degree at most `2`.
///
/// The coefficients `{q_M}_{i,j}, q_i,q_c` of the polynomial are known
/// The coefficients `{q_M}_{i,j}, q_i, constant` of the polynomial are known
/// values which define the opcode.
///
/// A general expression of assert-zero opcode is the following:
/// ```text
/// \sum_{i,j} {q_M}_{i,j}w_iw_j + \sum_i q_iw_i +q_c = 0
/// \sum_{i,j} {q_M}_{i,j}w_iw_j + \sum_i q_iw_i + constant = 0
/// ```
///
/// An assert-zero opcode can be used to:
Expand Down Expand Up @@ -140,7 +140,7 @@ impl<F: AcirField> std::fmt::Display for Opcode<F> {
for i in &expr.linear_combinations {
write!(f, "({}, _{}) ", i.0, i.1.witness_index())?;
}
write!(f, "{}", expr.q_c)?;
write!(f, "{}", expr.constant)?;

write!(f, " ]")
}
Expand Down
31 changes: 15 additions & 16 deletions acvm-repo/acir/src/native_types/expression/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,13 @@ pub struct Expression<F> {
pub mul_terms: Vec<(F, Witness, Witness)>,

pub linear_combinations: Vec<(F, Witness)>,
// TODO: rename q_c to `constant` moreover q_X is not clear to those who
// TODO are not familiar with PLONK
pub q_c: F,
// This is the constant term in the expression
pub constant: F,
}

impl<F: AcirField> Default for Expression<F> {
fn default() -> Self {
Expression { mul_terms: Vec::new(), linear_combinations: Vec::new(), q_c: F::zero() }
Expression { mul_terms: Vec::new(), linear_combinations: Vec::new(), constant: F::zero() }
}
}

Expand Down Expand Up @@ -78,7 +77,7 @@ impl<F> Expression<F> {
/// - f(x,y) = x + y would return `None`
/// - f(x,y) = 5 would return `FieldElement(5)`
pub fn to_const(&self) -> Option<&F> {
self.is_const().then_some(&self.q_c)
self.is_const().then_some(&self.constant)
}

/// Returns `true` if highest degree term in the expression is one or less.
Expand Down Expand Up @@ -128,8 +127,8 @@ impl<F> Expression<F> {
}

impl<F: AcirField> Expression<F> {
pub fn from_field(q_c: F) -> Self {
Self { q_c, ..Default::default() }
pub fn from_field(constant: F) -> Self {
Self { constant, ..Default::default() }
}

pub fn zero() -> Self {
Expand Down Expand Up @@ -157,7 +156,7 @@ impl<F: AcirField> Expression<F> {
// ie where the constant term is 0 and the coefficient in front of the variable is
// one.
let (coefficient, variable) = self.linear_combinations[0];
let constant = self.q_c;
let constant = self.constant;

if coefficient.is_one() && constant.is_zero() {
return Some(variable);
Expand All @@ -172,16 +171,16 @@ impl<F: AcirField> Expression<F> {
return self.clone();
} else if self.is_const() {
let kb = b * k;
return kb + self.q_c;
return kb + self.constant;
} else if b.is_const() {
return self.clone() + (k * b.q_c);
return self.clone() + (k * b.constant);
}

let mut mul_terms: Vec<(F, Witness, Witness)> =
Vec::with_capacity(self.mul_terms.len() + b.mul_terms.len());
let mut linear_combinations: Vec<(F, Witness)> =
Vec::with_capacity(self.linear_combinations.len() + b.linear_combinations.len());
let q_c = self.q_c + k * b.q_c;
let constant = self.constant + k * b.constant;

//linear combinations
let mut i1 = 0; //a
Expand Down Expand Up @@ -272,7 +271,7 @@ impl<F: AcirField> Expression<F> {
i2 += 1;
}

Expression { mul_terms, linear_combinations, q_c }
Expression { mul_terms, linear_combinations, constant }
}

/// Determine the width of this expression.
Expand Down Expand Up @@ -332,7 +331,7 @@ impl<F: AcirField> Expression<F> {

impl<F: AcirField> From<F> for Expression<F> {
fn from(constant: F) -> Self {
Expression { q_c: constant, linear_combinations: Vec::new(), mul_terms: Vec::new() }
Expression { constant, linear_combinations: Vec::new(), mul_terms: Vec::new() }
}
}

Expand All @@ -344,7 +343,7 @@ impl<F: AcirField> From<Witness> for Expression<F> {
/// can be seen as a univariate polynomial
fn from(wit: Witness) -> Self {
Expression {
q_c: F::zero(),
constant: F::zero(),
linear_combinations: vec![(F::one(), wit)],
mul_terms: Vec::new(),
}
Expand Down Expand Up @@ -372,7 +371,7 @@ mod tests {
(FieldElement::from(4u128), Witness(4), Witness(5)),
],
linear_combinations: vec![(FieldElement::from(4u128), Witness(4))],
q_c: FieldElement::one(),
constant: FieldElement::one(),
};

let result = a.add_mul(k, &b);
Expand All @@ -385,7 +384,7 @@ mod tests {
(FieldElement::from(40u128), Witness(4), Witness(5)),
],
linear_combinations: vec![(FieldElement::from(40u128), Witness(4))],
q_c: FieldElement::from(10u128)
constant: FieldElement::from(10u128)
}
);
}
Expand Down
56 changes: 30 additions & 26 deletions acvm-repo/acir/src/native_types/expression/operators.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,20 @@ use super::Expression;
impl<F: AcirField> Neg for &Expression<F> {
type Output = Expression<F>;
fn neg(self) -> Self::Output {
// XXX(med) : Implement an efficient way to do this
let mut mul_terms = self.mul_terms.clone();
let mut linear_combinations = self.linear_combinations.clone();

let mul_terms: Vec<_> =
self.mul_terms.iter().map(|(q_m, w_l, w_r)| (-*q_m, *w_l, *w_r)).collect();
for (coeff, _, _) in mul_terms.iter_mut() {
*coeff = -*coeff;
}

for (coeff, _) in linear_combinations.iter_mut() {
*coeff = -*coeff;
}

let linear_combinations: Vec<_> =
self.linear_combinations.iter().map(|(q_k, w_k)| (-*q_k, *w_k)).collect();
let q_c = -self.q_c;
let constant = -self.constant;

Expression { mul_terms, linear_combinations, q_c }
Expression { mul_terms, linear_combinations, constant }
}
}

Expand All @@ -31,19 +35,19 @@ impl<F: AcirField> Add<F> for Expression<F> {
type Output = Self;
fn add(self, rhs: F) -> Self::Output {
// Increase the constant
let q_c = self.q_c + rhs;
let constant = self.constant + rhs;

Expression { mul_terms: self.mul_terms, q_c, linear_combinations: self.linear_combinations }
Expression { mul_terms: self.mul_terms, constant, linear_combinations: self.linear_combinations }
}
}

impl<F: AcirField> Sub<F> for Expression<F> {
type Output = Self;
fn sub(self, rhs: F) -> Self::Output {
// Increase the constant
let q_c = self.q_c - rhs;
let constant = self.constant - rhs;

Expression { mul_terms: self.mul_terms, q_c, linear_combinations: self.linear_combinations }
Expression { mul_terms: self.mul_terms, constant, linear_combinations: self.linear_combinations }
}
}

Expand All @@ -59,9 +63,9 @@ impl<F: AcirField> Mul<F> for &Expression<F> {
self.linear_combinations.iter().map(|(q_l, w_l)| (*q_l * rhs, *w_l)).collect();

// Scale the constant
let q_c = self.q_c * rhs;
let constant = self.constant * rhs;

Expression { mul_terms, q_c, linear_combinations: lin_combinations }
Expression { mul_terms, constant, linear_combinations: lin_combinations }
}
}

Expand Down Expand Up @@ -119,16 +123,16 @@ impl<F: AcirField> Mul<&Expression<F>> for &Expression<F> {
type Output = Option<Expression<F>>;
fn mul(self, rhs: &Expression<F>) -> Self::Output {
if self.is_const() {
return Some(rhs * self.q_c);
return Some(rhs * self.constant);
} else if rhs.is_const() {
return Some(self * rhs.q_c);
return Some(self * rhs.constant);
} else if !(self.is_linear() && rhs.is_linear()) {
// `Expression`s can only represent terms which are up to degree 2.
// We then disallow multiplication of `Expression`s which have degree 2 terms.
return None;
}

let mut output = Expression::from_field(self.q_c * rhs.q_c);
let mut output = Expression::from_field(self.constant * rhs.constant);

//TODO to optimize...
for lc in &self.linear_combinations {
Expand All @@ -144,8 +148,8 @@ impl<F: AcirField> Mul<&Expression<F>> for &Expression<F> {
let (b_c, b_w) = rhs.linear_combinations[i2];

// Apply scaling from multiplication
let a_c = rhs.q_c * a_c;
let b_c = self.q_c * b_c;
let a_c = rhs.constant * a_c;
let b_c = self.constant * b_c;

let (coeff, witness) = match a_w.cmp(&b_w) {
Ordering::Greater => {
Expand All @@ -171,15 +175,15 @@ impl<F: AcirField> Mul<&Expression<F>> for &Expression<F> {
}
while i1 < self.linear_combinations.len() {
let (a_c, a_w) = self.linear_combinations[i1];
let coeff = rhs.q_c * a_c;
let coeff = rhs.constant * a_c;
if !coeff.is_zero() {
output.linear_combinations.push((coeff, a_w));
}
i1 += 1;
}
while i2 < rhs.linear_combinations.len() {
let (b_c, b_w) = rhs.linear_combinations[i2];
let coeff = self.q_c * b_c;
let coeff = self.constant * b_c;
if !coeff.is_zero() {
output.linear_combinations.push((coeff, b_w));
}
Expand Down Expand Up @@ -215,13 +219,13 @@ mod tests {
let a = Expression {
mul_terms: vec![],
linear_combinations: vec![(FieldElement::from(2u128), Witness(2))],
q_c: FieldElement::from(2u128),
constant: FieldElement::from(2u128),
};

let b = Expression {
mul_terms: vec![],
linear_combinations: vec![(FieldElement::from(4u128), Witness(4))],
q_c: FieldElement::one(),
constant: FieldElement::one(),
};

assert_eq!(
Expand All @@ -232,7 +236,7 @@ mod tests {
(FieldElement::from(2u128), Witness(2)),
(FieldElement::from(4u128), Witness(4))
],
q_c: FieldElement::from(3u128)
constant: FieldElement::from(3u128)
}
);

Expand All @@ -245,13 +249,13 @@ mod tests {
let a = Expression {
mul_terms: vec![],
linear_combinations: vec![(FieldElement::from(2u128), Witness(2))],
q_c: FieldElement::from(2u128),
constant: FieldElement::from(2u128),
};

let b = Expression {
mul_terms: vec![],
linear_combinations: vec![(FieldElement::from(4u128), Witness(4))],
q_c: FieldElement::one(),
constant: FieldElement::one(),
};

assert_eq!(
Expand All @@ -262,7 +266,7 @@ mod tests {
(FieldElement::from(2u128), Witness(2)),
(FieldElement::from(8u128), Witness(4))
],
q_c: FieldElement::from(2u128)
constant: FieldElement::from(2u128)
}
);

Expand Down
2 changes: 1 addition & 1 deletion acvm-repo/acvm/src/compiler/optimizers/redundant_range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ impl<F: AcirField> RangeOptimizer<F> {
// as a range opcode for the number of bits required to hold that value.
if expr.is_degree_one_univariate() {
let (k, witness) = expr.linear_combinations[0];
let constant = expr.q_c;
let constant = expr.constant;
let witness_value = -constant / k;

if witness_value.is_zero() {
Expand Down
2 changes: 1 addition & 1 deletion acvm-repo/acvm/src/compiler/transformers/csat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ impl CSatTransformer {
// Add the rest of the elements back into the new_opcode
new_opcode.mul_terms.extend(opcode.mul_terms);
new_opcode.linear_combinations.extend(opcode.linear_combinations);
new_opcode.q_c = opcode.q_c;
new_opcode.constant = opcode.constant;
new_opcode.sort();
new_opcode
}
Expand Down
Loading