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

Implement Polynomial for MultilinearExtension #691

Merged
merged 11 commits into from
Oct 25, 2023
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@

### Features

- [\#691](https://github.com/arkworks-rs/algebra/pull/691) (`ark-poly`) Implement `Polynomial` for `SparseMultilinearExtension` and `DenseMultilinearExtension`.

### Improvements

### Bugfixes
Expand Down
17 changes: 16 additions & 1 deletion poly/src/evaluations/multivariate/multilinear/dense.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
//! Multilinear polynomial represented in dense evaluation form.

use crate::evaluations::multivariate::multilinear::{swap_bits, MultilinearExtension};
use crate::{
evaluations::multivariate::multilinear::{swap_bits, MultilinearExtension},
Polynomial,
};
use ark_ff::{Field, Zero};
use ark_serialize::{CanonicalDeserialize, CanonicalSerialize};
use ark_std::{
Expand Down Expand Up @@ -307,6 +310,18 @@ impl<F: Field> Zero for DenseMultilinearExtension<F> {
}
}

impl<F: Field> Polynomial<F> for DenseMultilinearExtension<F> {
type Point = Vec<F>;

fn degree(&self) -> usize {
1
}
Copy link
Member

Choose a reason for hiding this comment

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

Ditto


fn evaluate(&self, point: &Self::Point) -> F {
MultilinearExtension::<F>::evaluate(self, point).unwrap()
}
}

#[cfg(test)]
mod tests {
use crate::{DenseMultilinearExtension, MultilinearExtension};
Expand Down
3 changes: 3 additions & 0 deletions poly/src/evaluations/multivariate/multilinear/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ use ark_ff::{Field, Zero};
use ark_serialize::{CanonicalDeserialize, CanonicalSerialize};
use ark_std::rand::Rng;

use crate::Polynomial;

/// This trait describes an interface for the multilinear extension
/// of an array.
/// The latter is a multilinear polynomial represented in terms of its
Expand All @@ -39,6 +41,7 @@ pub trait MultilinearExtension<F: Field>:
+ for<'a> AddAssign<(F, &'a Self)>
+ for<'a> SubAssign<&'a Self>
+ Index<usize>
+ Polynomial<F, Point = Vec<F>>
{
/// Returns the number of variables in `self`
fn num_vars(&self) -> usize;
Expand Down
14 changes: 13 additions & 1 deletion poly/src/evaluations/multivariate/multilinear/sparse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

use crate::{
evaluations::multivariate::multilinear::swap_bits, DenseMultilinearExtension,
MultilinearExtension,
MultilinearExtension, Polynomial,
};
use ark_ff::{Field, Zero};
use ark_serialize::{CanonicalDeserialize, CanonicalSerialize};
Expand Down Expand Up @@ -227,6 +227,18 @@ impl<F: Field> Index<usize> for SparseMultilinearExtension<F> {
}
}

impl<F: Field> Polynomial<F> for SparseMultilinearExtension<F> {
type Point = Vec<F>;

fn degree(&self) -> usize {
Copy link
Member

Choose a reason for hiding this comment

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

It seems that elsewhere in the crate, we assume that degree here represents total degree, and not individual degree. This is in agreement with mathematical convention.

So this should be changed to return self.num_vars, and it might be worth adding an individual_degree method to Polynomial that would return the degree for univariate polynomials, 1 for multilinear polynomials, and the actual individual degree for general multivariate polynomials.

Copy link
Member Author

Choose a reason for hiding this comment

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

I adapted this to represent the total degree. I've also created a separate issue to discuss the exact design and the need for individual_degree: #692.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this method meant to represent a total degree bound, or the actual total degree of the particular instance this is being called on? For instance, the polynomial f(x, y, z) = xy + yz has num_vars equal to 3, but actual total degree 2.

Returning the actual total degree would be simple in the sparse case, but might require some work in the dense case if the polynomial is stored in evaluation form.

Incidentally, this caveat would also have applied to the code as it was before @Pratyush 's comment: we were always returning 1, but in the fringe case of constant polynomials, that did not match the total degree.

1
}

fn evaluate(&self, point: &Self::Point) -> F {
MultilinearExtension::<F>::evaluate(self, point).unwrap()
}
}

impl<F: Field> Add for SparseMultilinearExtension<F> {
type Output = SparseMultilinearExtension<F>;

Expand Down
Loading