Add div and divrem with rounding argument #1955
Conversation
We add the standard Julia notation
`div(5, 2, RoundUp)`
also for the type `ZZRingElem`, so that we can write
`div(ZZ(5), ZZ(2), RoundUp)`.
Fix previous bug: before, there was the line: ``` Base.divrem(x::ZZRingElem, y::Int) = (Base.div(x, y), Base.rem(x, y)) ``` But that is inefficient. The whole point of having a `divrem` function is to find the dividend and the remainder together. I have changed the above to ``` Base.divrem(x::ZZRingElem, y::Integer) = Base.divrem(x, ZZ(y)) ```
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1955 +/- ##
==========================================
- Coverage 87.94% 87.94% -0.01%
==========================================
Files 99 99
Lines 36416 36426 +10
==========================================
+ Hits 32027 32034 +7
- Misses 4389 4392 +3 ☔ View full report in Codecov by Sentry. |
|
@lgoettgens since you had a look at those recently, does this make sense? I think so, but it would be good for someone to confirm. |
There was a problem hiding this comment.
Looks fine mathematically, thanks @paemurru! I just added some suggestions to condense all of the redirection/coercion dispatches with a rounding mode to not consider each and only rounding mode. I know that we don't support all of julias rounding modes, but it will still error once a "real working" function is reached, and it makes it easier to add more rounding modes. Besides that some small formatting suggestions to keep the style more consistent.
Before merging this, I think it would be great if you could add some simple tests that exercise all of these new functions. It doesn't need to be something elaborate or so, but maybe just one pair of inputs that differed between the different rounding modes, and then you input it in different data types and verify that the result is sane.
Co-authored-by: Lars Göttgens <lars.goettgens@gmail.com>
Co-authored-by: Lars Göttgens <lars.goettgens@gmail.com>
We add the standard Julia notation
div(5, 2, RoundUp)also for the type
ZZRingElem, so that we can writediv(ZZ(5), ZZ(2), RoundUp).Also, we fix a bug. Namely, before there was the line:
But that is inefficient. The whole point of having a
divremfunction is to find the dividend and the remainder together. I have changed the above to