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

Tracking issue: core functionality for Miden VM constraints #75

Closed
5 of 6 tasks
Tracked by #79
grjte opened this issue Nov 21, 2022 · 10 comments
Closed
5 of 6 tasks
Tracked by #79

Tracking issue: core functionality for Miden VM constraints #75

grjte opened this issue Nov 21, 2022 · 10 comments
Assignees
Labels

Comments

@grjte
Copy link
Contributor

grjte commented Nov 21, 2022

Goal(s)

  • functionality for defining ~90% of constraints for Miden VM and Polygon Zero
  • codegen for hardware acceleration
  • improved testing for code correctness

Details

Functionality for Miden VM & Polygon Zero

In order to define the constraints on the stack overflow column in Miden VM we need a way to declare public input vectors of unknown size

We need to make the process of defining boundary constraints more flexible for Polygon Zero. We can do this by making it possible for them to define lagrange polynomials directly. This requires:

  • enabling declaration of validity constraints (constraints against a single row that don't follow the boundary constraint convenience syntax used by Miden VM)
  • giving access to the x value in each row via a new built-in $x

Codegen for hardware acceleration

  1. Add Sub to the IR
  2. Implement codegen option for generic constraint evaluation format #56

Improved testing

We need unit tests for the IR and the codegen modules. We should come up with a good setup (possibly using an external tool) that keeps our tests simple and readable and can be used for all codegen cases.

Tasks

Preview Give feedback
  1. IR v0.2
    Overcastan
  2. grjte
  3. enhancement v0.2
    grjte
  4. IR parser v0.2
    tohrnii

Tasks

Preview Give feedback
  1. IR good first issue
    Overcastan
  2. codegen
    Overcastan

Working group:

@Al-Kindi-0, @grjte, @Overcastan, @tohrnii

Workflow
  • Discussion should happen here or in the related sub-issues.
  • PRs should only be merged by the coordinator, to ensure everyone is able to review.
  • Aim to complete reviews within 24 hours.
  • When a related sub-issue is opened:
    • add it to the list of sub-issues in this tracking issue
  • When opening a related PR:
    • request review from everyone in this working group
  • When a sub-issue is completed:
    • close the related issue with a comment that links to the PR where the work was completed

Coordinator: @tohrnii

The working group coordinator ensures scope & progress tracking are transparent and accurate. They will:

  • Merge approved PRs after all working group members have completed their reviews.
    • add the PR # to the relevant section of the current tracking PR.
    • close any completed sub-issue(s) with a comment that links to the PR where the work was completed
  • Monitor workflow items and complete anything that slips through the cracks.
  • Monitor scope to see if anything is untracked or unclear. Create missing sub-issues or initiate discussion as required.
  • Monitor progress to see if there's anything which isn't moving forward. Initiate discussion as required.
  • Identify PRs with especially significant changes and add @grjte and @bobbinth for review.
@grjte grjte changed the title Tracking issue: AirScript v0.2 Tracking issue: core functionality for Miden VM constraints Nov 21, 2022
@bobbinth
Copy link
Contributor

In order to define the constraints on the stack overflow column in Miden VM we need a way to declare public input vectors of unknown size

One thing I realized is that I don't think this will be enough. Assuming we have such vectors, we won't be able to use them since we can't iterate over all elements in such vectors. To address this, we'll need to implement iterators.

@bobbinth
Copy link
Contributor

  • enabling declaration of validity constraints (constraints against a single row that don't follow the boundary constraint convenience syntax used by Miden VM)

This might be as simple as renaming transition_constraints into integrity_constraints (or something similar) as described in #53. In the IR, we should still probably have a single algebraic DAG, but we could mark entry points as transition vs. validity constraints.

@grjte
Copy link
Contributor Author

grjte commented Nov 22, 2022

This might be as simple as renaming transition_constraints into integrity_constraints (or something similar) as described in #53. In the IR, we should still probably have a single algebraic DAG, but we could mark entry points as transition vs. validity constraints.

Agreed about the single DAG & continuing the discussion from #53. We just need to separate #53 into 2 different issues and then we can add the issue for validity constraints here & move this discussion into it. I think @tohrnii is doing that while adding/updating the other sub-issues we need for this task

@grjte
Copy link
Contributor Author

grjte commented Nov 29, 2022

Two points came up for discussion in PR #63

  1. Do we want to use column-major or row-major form for matrices? (reference)
  2. Having constants in uppercase looks a bit strange inside the source section. Alternatively, we could define constants at the top of the file instead of having a separate source section. Something like the following:
const EXAMPLE = 5

@tohrnii
Copy link
Contributor

tohrnii commented Nov 29, 2022

@grjte There's #89 to track 2 and bobbinth also likes this approach. This also makes me wonder if we should also decide on a place to discuss issues that come up during reviews. Maybe we should do that in the corresponding sub-issue and then only open a new issue for that if it doesn't get resolved only through discussions there. What do you think?

@grjte
Copy link
Contributor Author

grjte commented Dec 1, 2022

This also makes me wonder if we should also decide on a place to discuss issues that come up during reviews. Maybe we should do that in the corresponding sub-issue and then only open a new issue for that if it doesn't get resolved only through discussions there. What do you think?

Yes, discussing in the corresponding sub-issue makes sense, let's do that! I meant to put it in the corresponding tracking issue (#77), but accidentally put it in here instead, sorry!

@Dominik1999
Copy link

Was there any progress here last week? Do you need anything to be unblocked?

@tohrnii
Copy link
Contributor

tohrnii commented Dec 6, 2022

@Dominik1999 Issue 54 was completed last week and 56 will probably be completed this week. Nothing seems to be a blocker at the moment.

@Dominik1999
Copy link

@tohrnii still all good here?

@tohrnii
Copy link
Contributor

tohrnii commented Dec 13, 2022

@Dominik1999 I think we should be able to complete most of the must haves by end of next week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Status: Done
Development

No branches or pull requests

6 participants