From 551c782b1bf388f07842af6a326963be00632d66 Mon Sep 17 00:00:00 2001 From: adria0 Date: Wed, 7 Dec 2022 12:35:30 +0100 Subject: [PATCH 1/3] - Implements `PartialOrd` for `Value` - Adds a `transpose` method to turn `Value>` into `Result>` - `Expression::identifier()` remove string memory reallocation --- halo2_proofs/src/circuit/value.rs | 39 ++++++++++++++ halo2_proofs/src/plonk/circuit.rs | 85 +++++++++++++++++++++---------- 2 files changed, 96 insertions(+), 28 deletions(-) diff --git a/halo2_proofs/src/circuit/value.rs b/halo2_proofs/src/circuit/value.rs index e6ae26cd..a6387a0f 100644 --- a/halo2_proofs/src/circuit/value.rs +++ b/halo2_proofs/src/circuit/value.rs @@ -612,6 +612,45 @@ impl Mul for Value<&Assigned> { } } +impl PartialEq> for Value { + fn eq(&self, other: &Value) -> bool { + if let Some((a, b)) = self.inner.as_ref().zip(other.inner.as_ref()) { + a.eq(b) + } else { + self.inner.is_none() == other.inner.is_none() + } + } +} +impl Eq for Value {} + +impl PartialOrd> for Value { + fn partial_cmp(&self, other: &Value) -> Option { + match (self.inner.as_ref(), other.inner.as_ref()) { + (None, None) => Some(std::cmp::Ordering::Equal), + (None, Some(_)) => Some(std::cmp::Ordering::Less), + (Some(_), None) => Some(std::cmp::Ordering::Greater), + (Some(a), Some(b)) => a.partial_cmp(b), + } + } +} + +impl Ord for Value { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + self.partial_cmp(other).unwrap() + } +} + +impl Value> { + /// Transposes an `Value` of a [`Result`] into a [`Result`] of an `Value`. + pub fn transpose(self) -> Result, E> { + match self.inner { + Some(Ok(x)) => Ok(Value::known(x)), + Some(Err(e)) => Err(e), + None => Ok(Value::unknown()), + } + } +} + impl Value { /// Returns the field element corresponding to this value. pub fn to_field(&self) -> Value> diff --git a/halo2_proofs/src/plonk/circuit.rs b/halo2_proofs/src/plonk/circuit.rs index 7ad8d8b8..a512c28f 100644 --- a/halo2_proofs/src/plonk/circuit.rs +++ b/halo2_proofs/src/plonk/circuit.rs @@ -925,38 +925,67 @@ impl Expression { } } + fn write_identifier(&self, writer: &mut W) -> std::io::Result<()> { + match self { + Expression::Constant(scalar) => write!(writer, "{:?}", scalar), + Expression::Selector(selector) => write!(writer, "selector[{}]", selector.0), + Expression::Fixed(query) => { + write!( + writer, + "fixed[{}][{}]", + query.column_index, query.rotation.0 + ) + } + Expression::Advice(query) => { + write!( + writer, + "advice[{}][{}]", + query.column_index, query.rotation.0 + ) + } + Expression::Instance(query) => { + write!( + writer, + "instance[{}][{}]", + query.column_index, query.rotation.0 + ) + } + Expression::Challenge(challenge) => { + write!(writer, "challenge[{}]", challenge.index()) + } + Expression::Negated(a) => { + writer.write_all(b"(-")?; + a.write_identifier(writer)?; + writer.write_all(b")") + } + Expression::Sum(a, b) => { + writer.write_all(b"(")?; + a.write_identifier(writer)?; + writer.write_all(b"+")?; + b.write_identifier(writer)?; + writer.write_all(b")") + } + Expression::Product(a, b) => { + writer.write_all(b"(")?; + a.write_identifier(writer)?; + writer.write_all(b"*")?; + b.write_identifier(writer)?; + writer.write_all(b")") + } + Expression::Scaled(a, f) => { + a.write_identifier(writer)?; + write!(writer, "*{:?}", f) + } + } + } + /// Identifier for this expression. Expressions with identical identifiers /// do the same calculation (but the expressions don't need to be exactly equal /// in how they are composed e.g. `1 + 2` and `2 + 1` can have the same identifier). pub fn identifier(&self) -> String { - match self { - Expression::Constant(scalar) => format!("{:?}", scalar), - Expression::Selector(selector) => format!("selector[{}]", selector.0), - Expression::Fixed(query) => { - format!("fixed[{}][{}]", query.column_index, query.rotation.0) - } - Expression::Advice(query) => { - format!("advice[{}][{}]", query.column_index, query.rotation.0) - } - Expression::Instance(query) => { - format!("instance[{}][{}]", query.column_index, query.rotation.0) - } - Expression::Challenge(challenge) => { - format!("challenge[{}]", challenge.index()) - } - Expression::Negated(a) => { - format!("(-{})", a.identifier()) - } - Expression::Sum(a, b) => { - format!("({}+{})", a.identifier(), b.identifier()) - } - Expression::Product(a, b) => { - format!("({}*{})", a.identifier(), b.identifier()) - } - Expression::Scaled(a, f) => { - format!("{}*{:?}", a.identifier(), f) - } - } + let mut cursor = std::io::Cursor::new(Vec::new()); + self.write_identifier(&mut cursor).unwrap(); + String::from_utf8(cursor.into_inner()).unwrap() } /// Compute the degree of this polynomial From e56a4a4f1e0cd75c2afc4ace53b5a5bb13a88224 Mon Sep 17 00:00:00 2001 From: adria0 Date: Mon, 9 Jan 2023 15:06:45 +0100 Subject: [PATCH 2/3] Remove partial ordering for value --- halo2_proofs/src/circuit/value.rs | 28 ---------------------------- 1 file changed, 28 deletions(-) diff --git a/halo2_proofs/src/circuit/value.rs b/halo2_proofs/src/circuit/value.rs index a6387a0f..1fadce21 100644 --- a/halo2_proofs/src/circuit/value.rs +++ b/halo2_proofs/src/circuit/value.rs @@ -612,34 +612,6 @@ impl Mul for Value<&Assigned> { } } -impl PartialEq> for Value { - fn eq(&self, other: &Value) -> bool { - if let Some((a, b)) = self.inner.as_ref().zip(other.inner.as_ref()) { - a.eq(b) - } else { - self.inner.is_none() == other.inner.is_none() - } - } -} -impl Eq for Value {} - -impl PartialOrd> for Value { - fn partial_cmp(&self, other: &Value) -> Option { - match (self.inner.as_ref(), other.inner.as_ref()) { - (None, None) => Some(std::cmp::Ordering::Equal), - (None, Some(_)) => Some(std::cmp::Ordering::Less), - (Some(_), None) => Some(std::cmp::Ordering::Greater), - (Some(a), Some(b)) => a.partial_cmp(b), - } - } -} - -impl Ord for Value { - fn cmp(&self, other: &Self) -> std::cmp::Ordering { - self.partial_cmp(other).unwrap() - } -} - impl Value> { /// Transposes an `Value` of a [`Result`] into a [`Result`] of an `Value`. pub fn transpose(self) -> Result, E> { From 8c26a8309fa6769170368fc8310771b543c83a78 Mon Sep 17 00:00:00 2001 From: adria0 Date: Mon, 9 Jan 2023 16:49:38 +0100 Subject: [PATCH 3/3] Remove transpose --- halo2_proofs/src/circuit/value.rs | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/halo2_proofs/src/circuit/value.rs b/halo2_proofs/src/circuit/value.rs index 1fadce21..e6ae26cd 100644 --- a/halo2_proofs/src/circuit/value.rs +++ b/halo2_proofs/src/circuit/value.rs @@ -612,17 +612,6 @@ impl Mul for Value<&Assigned> { } } -impl Value> { - /// Transposes an `Value` of a [`Result`] into a [`Result`] of an `Value`. - pub fn transpose(self) -> Result, E> { - match self.inner { - Some(Ok(x)) => Ok(Value::known(x)), - Some(Err(e)) => Err(e), - None => Ok(Value::unknown()), - } - } -} - impl Value { /// Returns the field element corresponding to this value. pub fn to_field(&self) -> Value>