-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
[stdlib] Roundable trait with ndigits param in the round operation and Round to nearest even logic #2752
[stdlib] Roundable trait with ndigits param in the round operation and Round to nearest even logic #2752
Conversation
Signed-off-by: Manuel Saelices <msaelices@gmail.com>
…missing types Signed-off-by: Manuel Saelices <msaelices@gmail.com>
Signed-off-by: Manuel Saelices <msaelices@gmail.com>
Signed-off-by: Manuel Saelices <msaelices@gmail.com>
Signed-off-by: Manuel Saelices <msaelices@gmail.com>
Signed-off-by: Manuel Saelices <msaelices@gmail.com>
As the round() and Roundable trait was moved previously there Signed-off-by: Manuel Saelices <msaelices@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this! I have a few concerns with the Int
/IntLiteral
stuff, but looking good otherwise!
@msaelices Are you still working on this? This is a very nice improvement, and I would love to bring it in! |
Signed-off-by: Manuel Saelices <msaelices@gmail.com>
Signed-off-by: Manuel Saelices <msaelices@gmail.com>
…at runtime Signed-off-by: Manuel Saelices <msaelices@gmail.com>
Sure. I'm working on this now. |
Signed-off-by: Manuel Saelices <msaelices@gmail.com>
Signed-off-by: Manuel Saelices <msaelices@gmail.com>
@laszlokindrat could you please take another look? |
Signed-off-by: Manuel Saelices <msaelices@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great progress, thanks for addressing my reviews! It seems you could split this up into several independent patches for the different types. That would make it easier to land these. Then the patch for the trait could be added after they all landed, and it should be simple. It's up to you, but I think it would speed things up :)
@@ -269,6 +269,47 @@ struct IntLiteral( | |||
""" | |||
return self | |||
|
|||
@always_inline("nodebug") | |||
fn __divmod__(self, rhs: Self) -> Tuple[Self, Self]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
non-blocking: maybe in a followup: Maybe it's time for a DivModable
trait or something. Then we could hook these up into the builtin divmod
function.
@@ -1142,6 +1142,18 @@ struct SIMD[type: DType, size: Int = simdwidthof[type]()]( | |||
""" | |||
return llvm_intrinsic["llvm.round", Self, has_side_effect=False](self) | |||
|
|||
@always_inline("nodebug") | |||
fn __round__(self, ndigits: Int) -> Self: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
non-blocking: SIMD might benefit from having a separate overload that does this elementwise between two vectors. Might get tricky for the different dtypes, though. Anyway, just a followup idea.
I think I prefer not to split the PR. It would be harder for me to cherry-pick all the commits or redo my work and track all of them. |
Signed-off-by: Manuel Saelices <msaelices@gmail.com>
Signed-off-by: Manuel Saelices <msaelices@gmail.com>
Signed-off-by: Manuel Saelices <msaelices@gmail.com>
9247ca1
to
6039f4e
Compare
@laszlokindrat Could you please TAL? 🏓 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice, thank you for working on this!
!sync |
Signed-off-by: Manuel Saelices <msaelices@gmail.com>
!sync |
1 similar comment
!sync |
✅🟣 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. |
…peration and Round to nearest even logic (#40794) [External] [stdlib] Roundable trait with ndigits param in the round operation and Round to nearest even logic * Add `Roundable.__round__(ndigits)` method * Implement the method in all the types that adhere to `Roundable` * Round to the next even number when the diff is exactly 0.5, recommended way in [IEEE 754](https://en.wikipedia.org/wiki/IEEE_754), which also Python adheres to. * Tests --------- Co-authored-by: Manuel Saelices <msaelices@gmail.com> Closes #2752 MODULAR_ORIG_COMMIT_REV_ID: f3349ff968cdb519bd505427a6aea8afe1ae1bb0
Landed in 4edfc69! Thank you for your contribution 🎉 |
…peration and Round to nearest even logic (#40794) [External] [stdlib] Roundable trait with ndigits param in the round operation and Round to nearest even logic * Add `Roundable.__round__(ndigits)` method * Implement the method in all the types that adhere to `Roundable` * Round to the next even number when the diff is exactly 0.5, recommended way in [IEEE 754](https://en.wikipedia.org/wiki/IEEE_754), which also Python adheres to. * Tests --------- Co-authored-by: Manuel Saelices <msaelices@gmail.com> Closes #2752 MODULAR_ORIG_COMMIT_REV_ID: f3349ff968cdb519bd505427a6aea8afe1ae1bb0
Roundable.__round__(ndigits)
methodRoundable