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.

Sign up for GitHub

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

Jump to bottom

[dev] [optimization] Division and modulus operations to div_rem #206

Closed
b-j-roberts opened this issue Sep 11, 2024 · 9 comments
Closed

[dev] [optimization] Division and modulus operations to div_rem #206

b-j-roberts opened this issue Sep 11, 2024 · 9 comments
Assignees
@adrianvrj
Labels
enhancement New feature or request

Comments

@b-j-roberts
Copy link
Contributor

To optimize shinigami, we should be using div_rem instead of % and / when doing these operations on the same number. So for each instance in the code where modulus and division are used together, we should change them to use div_rem instead to improve efficiency.

References

Div rem example
Example of location to optimize in the code
@b-j-roberts b-j-roberts added the enhancement New feature or request label Sep 11, 2024
@adrianvrj
Copy link
Contributor

adrianvrj commented Sep 11, 2024 •
edited
Loading

hi @b-j-roberts can I tackle this one?
I have experience with the project and I am very interested in continuing to contribute

@onlydustapp OnlyDustApp
Copy link

onlydustapp bot commented Sep 11, 2024

Hey @adrianvrj!
Thanks for showing interest.
We've created an application for you to contribute to shinigami.
Go check it out on OnlyDust!

@mubarak23
Copy link
Contributor

@b-j-roberts optimizing the opcode is the best way to go, kindly assign

@ShantelPeters
Copy link
Contributor

Hi @b-j-roberts please can I work on this

@onlydustapp OnlyDustApp
Copy link

onlydustapp bot commented Sep 11, 2024

Hey @ShantelPeters!
Thanks for showing interest.
We've created an application for you to contribute to shinigami.
Go check it out on OnlyDust!

@manlikeHB
Copy link
Contributor

can i work on this?

@onlydustapp OnlyDustApp
Copy link

onlydustapp bot commented Sep 11, 2024

Hey @manlikeHB!
Thanks for showing interest.
We've created an application for you to contribute to shinigami.
Go check it out on OnlyDust!

@b-j-roberts
Copy link
Contributor Author

hi @b-j-roberts can I tackle this one? I have experience with the project and I am very interested in continuing to contribute

Sure, I will assign you and check back in a couple days. Please lmk if you have questions

@b-j-roberts
Copy link
Contributor Author

@adrianvrj Assigned, thank you!

@adrianvrj adrianvrj mentioned this issue Sep 16, 2024
Merged
3 tasks
b-j-roberts added a commit that referenced this issue Sep 18, 2024
@adrianvrj @b-j-roberts
[fix] used div_rem when using / and % in same value (#219)
4b14e3e 
<!-- enter the gh issue after hash -->

- [x] issue #206
- [x] follows contribution
[guide](https://github.com/keep-starknet-strange/shinigami/blob/main/CONTRIBUTING.md)
- [x] code change includes tests

<!-- PR description below -->
This PR will add the usage of div_rem when using / and % in the same
value.

*Pending:* in utils.cairo file, the ``` fast_power ``` function is
giving problems for using div_rem.

Co-authored-by: Brandon Roberts <brandonjroberts22@gmail.com>
j1mbo64 pushed a commit to j1mbo64/shinigami that referenced this issue Sep 18, 2024
@adrianvrj @b-j-roberts
[fix] used div_rem when using / and % in same value (keep-starknet-st…
304e614 
…range#219)

<!-- enter the gh issue after hash -->

- [x] issue keep-starknet-strange#206
- [x] follows contribution
[guide](https://github.com/keep-starknet-strange/shinigami/blob/main/CONTRIBUTING.md)
- [x] code change includes tests

<!-- PR description below -->
This PR will add the usage of div_rem when using / and % in the same
value.

*Pending:* in utils.cairo file, the ``` fast_power ``` function is
giving problems for using div_rem.

Co-authored-by: Brandon Roberts <brandonjroberts22@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@adrianvrj adrianvrj

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@mubarak23 @b-j-roberts @manlikeHB @adrianvrj @ShantelPeters