Python: fixed gen, lookup, and mimc7 #117

qwang98 · 2023-08-29T13:19:50Z

Ready for review @leolara.

Changes Related to Fixed Gen

To parse Python generated fixed assignments directly to Rust, I moved fixed assignments generation from compiler to dsl/ast
As a result, Rust ast no longer stores fixed_gen lambda, but is replaced with fixed_assignments
Added associated functions in Python
New (Incomplete) Python Example: Mimc7

Mimc7 example works for the mimc7_constants circuit

To support this example, Lookup is now enabled in Python frontend and can be parsed to Rust ast
The main circuit is still under development and will be pushed in another PR for Python super circuit
Won't close the Mimc7 PR till we have supercircuit done

leolara · 2023-08-30T04:51:56Z

Please constants at the bottom if python allows it, even better if the constants can be in another file.

qwang98 · 2023-08-30T13:16:40Z

Please constants at the bottom if python allows it, even better if the constants can be in another file.

Putting at bottom won't work, event with __future__ imported. I put it in another file.

leolara · 2023-08-30T13:59:45Z

src/frontend/python/chiquito/dsl.py

+                raise ValueError(
+                    "Must set num_steps by calling pragma_num_steps() in setup before calling fixed_gen()."
+                )
+            self.fixed_gen_context = FixedGenContext.new(self.ast.num_steps)


@qwang98 I don't think we need the class FixedGenContext anymore, we could keep the assignations array in this class

Agreed. I kept fixed_assignments in AST as we did in Rust, and added AST function that modifies it. DSL calls this AST function.

qwang98 · 2023-09-01T14:08:56Z

Addressed Leo's comments. Now still runs successfully on mimc7 example. Ready for review again. @leolara

New Python package version published as 2023090100

examples/mimc7_constants.py

leolara · 2023-09-01T07:17:31Z

src/ast/mod.rs

+}
+
+impl<F: Field + Hash, TraceArgs> Circuit<F, TraceArgs> {
+    pub fn set_fixed_assignments<D>(&mut self, def: D)


The name of the function suggest that you are just setting something but it runs the lambda with a context. I think running the lambda with the context is better done in dsl and here just a setter.

leolara · 2023-09-01T07:20:14Z

src/frontend/python/chiquito/cb.py

+        return self
+
+    def build(self: LookupTableBuilder, step_type: StepType) -> Lookup:
+        table = step_type.tables.get(self.id)


Why the table has to be stored in the step?

In Rust Chiquito, table is initially built using new_table from DSL and stored in CircuitContext. It's then cloned to StepTypeContext and StepTypeSetupContext. Because build is called by add_lookup, a StepTypeSetupContext function, build conveniently takes a ctx: StepTypeSetupContext parameter. Therefore, table doesn't "have to be" stored in the step, but it's just the most convenient for build to have the step_type parameter, as add_lookup calls build at the step type level.

In Python, add_lookup also calls build at the step type level, and that's why we have the step_type parameter here. The tables are initially created and stored at the circuit level, but step type and super circuit can also have references to the tables.

leolara · 2023-09-06T00:04:53Z

sorry, I had comments but I didn't submit them

qwang98 · 2023-09-10T17:19:42Z

Ready for review again. @leolara

@leolara

@leolara Ready for review. Note that the Fibo example won't be entirely fixed till we have #117 merged, because PyPI is already updated with an AST that requires `fixed_assignments` while this branch has an older AST.

leolara · 2023-09-16T12:15:56Z

src/ast/mod.rs

    pub fn get_step_type(&self, uuid: UUID) -> Rc<StepType<F>> {
        let step_rc = self.step_types.get(&uuid).expect("step type not found");

        Rc::clone(step_rc)
    }
 }

+impl<F: Field + Hash, TraceArgs> Circuit<F, TraceArgs> {
+    pub fn set_fixed_assignments(&mut self, assignments: Option<FixedAssignment<F>>) {


@qwang98 why is the parameter an option? Would you call it with None?

leolara · 2023-09-16T12:17:11Z

src/frontend/python/chiquito/cb.py

+
+
+@dataclass
+class LookupTableRegistry:


@qwang98 I don't think we need a LookupTableRegistry, this was necessary in rust to have the Copy constraint, but in python the table is a reference.

Removed and fixed!

leolara · 2023-09-16T12:28:19Z

src/frontend/python/chiquito/cb.py

+
+
+@dataclass
+class LookupTable:


@qwang98 when a table is created, it should be close to add more expresions to dest. So perhaps there is a method create() that returns self and sets a flag that prevents from adding more things to dest. Also, the table cannot be used if this flag has not been activated by call to create()

I believe this create() function you mentioned is actually new_table() in dsl.py. Rust Chiquito has the same naming.

I modified LookupTable such that now we have a new read_only flag, that defaults to False when LookupTable is just created. User can then call add() to add more destination columns to the LookupTable, which requires that read_only == False.

LookupTable will then need to be added by new_table() in dsl.py to the dictionary of UUID to LookupTable. new_table() also changes the read_only flag to True, so that no more calls of add() to modify the destination column is possible after new_table() is called.

Finally, any use of the table will be through apply() and when(), both of which requires that read_only == True.

This should accomplish all the features you suggested above.

What's more? create_table() can only be called during setup() and there's a flag assertion for that as well.

qwang98 · 2023-09-17T21:02:05Z

Everything fixed and ready for review again. @leolara

qwang98 · 2023-09-19T17:04:26Z

All contents will be PR'ed through #125 instead. Closing this.

qwang98 added 2 commits August 29, 2023 09:18

initial commit and mimc7 example

2d7027c

all updates

545d651

qwang98 requested a review from leolara August 29, 2023 13:30

qwang98 mentioned this pull request Aug 29, 2023

Steve/update pychiquito #107

Closed

qwang98 linked an issue Aug 29, 2023 that may be closed by this pull request

Refactor fixed assignments #90

Closed

qwang98 marked this pull request as ready for review August 29, 2023 17:06

qwang98 changed the title ~~initial commit and mimc7 example~~ Python: fixed gen, lookup, and mimc7 Aug 30, 2023

put mimc7 constants to anothe file

8ede657

leolara reviewed Aug 30, 2023

View reviewed changes

qwang98 added 3 commits September 1, 2023 09:31

merge main

c784a55

addressed leo's comments

4e937b1

updated wit_gen

98eb019

qwang98 requested a review from leolara September 1, 2023 14:09

leolara reviewed Sep 6, 2023

View reviewed changes

qwang98 added 4 commits September 10, 2023 12:22

address leo's comments

62c5fae

merge main

6428248

dsl

965b465

addressed all leo's comments

2a1905b

qwang98 requested a review from leolara September 10, 2023 17:19

fmt

02a3c24

This was referenced Sep 10, 2023

Python Supercircuit #125

Merged

Python: allow arbitrary args #132

Merged

leolara reviewed Sep 16, 2023

View reviewed changes

qwang98 added 4 commits September 17, 2023 16:37

fixed optional fixed assignments

76ef232

deleted registry

afe5177

final fixed

a768216

add flag to lookuptable

227476d

qwang98 requested a review from leolara September 17, 2023 21:02

qwang98 added 7 commits September 17, 2023 17:03

added features for lookuptable

5843774

merged main

e871c98

fix error

8cdbf27

fix more errors

f3f181e

minor

987029e

minor

f4c0092

final touch

6999d45

qwang98 closed this Sep 19, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Python: fixed gen, lookup, and mimc7 #117

Python: fixed gen, lookup, and mimc7 #117

qwang98 commented Aug 29, 2023 •

edited

Loading

leolara commented Aug 30, 2023

qwang98 commented Aug 30, 2023 •

edited

Loading

leolara Aug 30, 2023

qwang98 Sep 1, 2023

qwang98 commented Sep 1, 2023 •

edited

Loading

leolara Sep 1, 2023

qwang98 Sep 10, 2023

leolara Sep 1, 2023

qwang98 Sep 10, 2023 •

edited

Loading

leolara commented Sep 6, 2023

qwang98 commented Sep 10, 2023

leolara Sep 16, 2023

qwang98 Sep 17, 2023

leolara Sep 16, 2023

qwang98 Sep 17, 2023

leolara Sep 16, 2023

qwang98 Sep 17, 2023

qwang98 commented Sep 17, 2023

qwang98 commented Sep 19, 2023

Python: fixed gen, lookup, and mimc7 #117

Python: fixed gen, lookup, and mimc7 #117

Conversation

qwang98 commented Aug 29, 2023 • edited Loading

leolara commented Aug 30, 2023

qwang98 commented Aug 30, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qwang98 commented Sep 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qwang98 Sep 10, 2023 • edited Loading

Choose a reason for hiding this comment

leolara commented Sep 6, 2023

qwang98 commented Sep 10, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qwang98 commented Sep 17, 2023

qwang98 commented Sep 19, 2023

qwang98 commented Aug 29, 2023 •

edited

Loading

qwang98 commented Aug 30, 2023 •

edited

Loading

qwang98 commented Sep 1, 2023 •

edited

Loading

qwang98 Sep 10, 2023 •

edited

Loading