Skip to content
This repository has been archived by the owner on Nov 4, 2024. It is now read-only.

Generalize fibonacci rust example - Issue #128 #131

Conversation

jae-cuz
Copy link
Contributor

@jae-cuz jae-cuz commented Sep 14, 2023

Resolves #128.

@leolara @qwang98 Ready for review. Added comments for added functionalities.

One thing to ask:
If we add a new file fibo_with_padding.rs, should we erase the same comments from fibonacci.rs? Please advise. Thanks!

@jae-cuz jae-cuz marked this pull request as ready for review September 15, 2023 06:36
// Fibonacci sequence is 10. (one less than num_steps above)

// Expose the result of calculation and round number
ctx.expose(b, chiquito::ast::ExposeOffset::Last);
Copy link
Collaborator

Choose a reason for hiding this comment

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

@jae-cuz do a use chiquito::ast::ExposeOffset

@leolara leolara requested a review from qwang98 September 15, 2023 07:42
@leolara
Copy link
Collaborator

leolara commented Sep 16, 2023

@qwang98 you are the reviewer of this

@qwang98
Copy link
Collaborator

qwang98 commented Sep 17, 2023

@leolara @qwang98 Ready for review. Added comments for added functionalities.

One thing to ask: If we add a new file fibo_with_padding.rs, should we erase the same comments from fibonacci.rs? Please advise. Thanks!

I agree that we should remove repetitive comments from fibonacci.rs but mention at the top of fibo_with_padding.rs that users should go through fibonacci.rs first.

Copy link
Collaborator

@qwang98 qwang98 left a comment

Choose a reason for hiding this comment

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

Please just fix the comments as suggested and I'm good with the PR otherwise.

@jae-cuz jae-cuz requested a review from qwang98 September 18, 2023 04:52
@qwang98
Copy link
Collaborator

qwang98 commented Sep 18, 2023

I'm good with this!

@qwang98 qwang98 added this pull request to the merge queue Sep 18, 2023
Merged via the queue into privacy-scaling-explorations:main with commit fdc0d7f Sep 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update rust fibo example
3 participants