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

[stdlib] add benchmark for lcm and gcd #3041

Closed
wants to merge 1 commit into from
Closed

[stdlib] add benchmark for lcm and gcd #3041

wants to merge 1 commit into from

Conversation

bgreni
Copy link
Contributor

@bgreni bgreni commented Jun 14, 2024 •
edited
Loading

Add benchmarks for lcm and gcd functions

@bgreni bgreni requested a review from a team as a code owner June 14, 2024 02:46
JoeLoser
JoeLoser reviewed Jun 14, 2024
View reviewed changes
Copy link
Collaborator

@JoeLoser JoeLoser left a comment

Choose a reason for hiding this comment

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

!sync

@JoeLoser
Copy link
Collaborator

!sync

@modularbot modularbot added the imported-internally Signals that a given pull request has been imported internally. label Jun 14, 2024
@bgreni bgreni force-pushed the add-lcm-gcd-benchmark branch 2 times, most recently from e1202b4 to 3a53b3a Compare June 17, 2024 21:57
@soraros

This comment was marked as resolved.

Sign in to view
@bgreni bgreni force-pushed the add-lcm-gcd-benchmark branch 2 times, most recently from 4b9a6c6 to 3e47510 Compare June 17, 2024 22:48
@bgreni bgreni force-pushed the add-lcm-gcd-benchmark branch 4 times, most recently from 1831c5c to afd0489 Compare July 2, 2024 22:34
@bgreni bgreni force-pushed the add-lcm-gcd-benchmark branch 2 times, most recently from a33a17b to b2d5887 Compare July 11, 2024 02:16
@bgreni bgreni force-pushed the add-lcm-gcd-benchmark branch from b2d5887 to a5b833b Compare July 16, 2024 22:52
modularbot pushed a commit that referenced this pull request Jul 17, 2024
@JoeLoser @modularbot
[stdlib] Don't make math.lcm take owned arguments
4227346 
There's no need for `math.lcm` to take ownership of its arguments.
Remove the `owned` keyword so the default argument convention of
`borrowed` is used for the function arguments. This will also simplify
#3041.

MODULAR_ORIG_COMMIT_REV_ID: 782c1016c7fe287fc21115504e75098431fe80dd
@bgreni bgreni force-pushed the add-lcm-gcd-benchmark branch from a5b833b to 397aed6 Compare July 17, 2024 20:10
@JoeLoser JoeLoser added the blocked Blocked by another issue that must be resolved first label Jul 17, 2024
@JoeLoser
Copy link
Collaborator

When importing this and playing with it (and adding a necessary benchmark.keep call to the function), it hit a compiler error. I filed an issue internally, but this is blocked until then.

@bgreni bgreni force-pushed the add-lcm-gcd-benchmark branch from 397aed6 to c0fd1d4 Compare August 4, 2024 20:16
@bgreni bgreni force-pushed the add-lcm-gcd-benchmark branch from c0fd1d4 to 3052543 Compare September 8, 2024 02:58
modularbot pushed a commit that referenced this pull request Sep 13, 2024
@JoeLoser @modularbot
[stdlib] Don't make math.lcm take owned arguments
2950935 
There's no need for `math.lcm` to take ownership of its arguments.
Remove the `owned` keyword so the default argument convention of
`borrowed` is used for the function arguments. This will also simplify
#3041.

MODULAR_ORIG_COMMIT_REV_ID: 782c1016c7fe287fc21115504e75098431fe80dd
@bgreni
add benchmark for lcm and gcd
35fcdce 
Signed-off-by: Brian Grenier <[email protected]>
@bgreni bgreni force-pushed the add-lcm-gcd-benchmark branch from 3052543 to 35fcdce Compare September 18, 2024 01:45
@modularbot
Copy link
Collaborator

✅🟣 This contribution has been merged 🟣✅

Your pull request has been merged to the internal upstream Mojo sources. It will be reflected here in the Mojo repository on the nightly branch during the next Mojo nightly release, typically within the next 24-48 hours.

We use Copybara to merge external contributions, click here to learn more.

@modularbot modularbot added merged-internally Indicates that this pull request has been merged internally merged-externally Merged externally in public mojo repo labels Oct 29, 2024
modularbot pushed a commit that referenced this pull request Oct 30, 2024
@bgreni @modularbot
[External] [stdlib] Add benchmark for lcm and gcd (#41811)
83bf0a3 
[External] [stdlib] Add benchmark for `lcm` and `gcd`

Add benchmarks for `lcm` and `gcd` functions.

Co-authored-by: bgreni <[email protected]>
Closes #3041
MODULAR_ORIG_COMMIT_REV_ID: 3fe9e4a6321832878a49ad79d90593ef2969076b
@modularbot
Copy link
Collaborator

Landed in 83bf0a3! Thank you for your contribution 🎉

@modularbot modularbot closed this Oct 30, 2024
Ahajha pushed a commit to Ahajha/mojo that referenced this pull request Oct 31, 2024
@bgreni @Ahajha
[External] [stdlib] Add benchmark for lcm and gcd (#41811)
2cc997d 
[External] [stdlib] Add benchmark for `lcm` and `gcd`

Add benchmarks for `lcm` and `gcd` functions.

Co-authored-by: bgreni <[email protected]>
Closes modularml#3041
MODULAR_ORIG_COMMIT_REV_ID: 3fe9e4a6321832878a49ad79d90593ef2969076b
modularbot pushed a commit that referenced this pull request Dec 17, 2024
@bgreni @modularbot
[External] [stdlib] Add benchmark for lcm and gcd (#41811)
85d204c 
[External] [stdlib] Add benchmark for `lcm` and `gcd`

Add benchmarks for `lcm` and `gcd` functions.

Co-authored-by: bgreni <[email protected]>
Closes #3041
MODULAR_ORIG_COMMIT_REV_ID: 3fe9e4a6321832878a49ad79d90593ef2969076b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JoeLoser JoeLoser JoeLoser left review comments

Assignees

@JoeLoser JoeLoser

Labels
blocked Blocked by another issue that must be resolved first imported-internally Signals that a given pull request has been imported internally. merged-externally Merged externally in public mojo repo merged-internally Indicates that this pull request has been merged internally
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bgreni @JoeLoser @soraros @modularbot