RFC: RoundNearest argument for rem #10946

simonbyrne · 2015-04-22T12:14:14Z

This is my attempt at resolving #9283. It adds an extra argument to rem, so that

rem(x,y,r::RoundingMode) = x - y*round(x/y,r)

without intermediate rounding, so that the result is always in the interval [-abs(y)/2,abs(y)/2]. This corresponds to the remainder function in the IEEE754 spec.

I've also implemented the corresponding rem2pi function, which is more accurate, and seems to be more useful, than mod2pi (I actually replaced the only 2 occurrences of mod2pi in Base, as this is what they were actually trying to get at).

If we're happy with this, I can add some docs, tests, and implement corresponding div functions.

simonbyrne · 2015-04-22T12:21:18Z

And I guess this would also be useful to have for integer arguments.

simonbyrne · 2015-04-22T19:59:59Z

Any suggestions for why the build is breaking on 32bit?

pao · 2015-04-22T21:29:46Z

Tuple overhaul related. Should be fixed by 7ec501d. Either force-push or get someone who can rerun the tests to trigger it and you should be good to go.

JeffBezanson · 2015-04-25T02:47:25Z

Looks really good; please continue!

StefanKarpinski · 2015-04-25T13:36:14Z

Yes, this is great. I like the design.

StefanKarpinski · 2017-01-04T18:26:38Z

@simonbyrne – this was generally well-liked, can we get it rebased and merged?

tkelman · 2017-01-04T20:04:33Z

base/exports.jl

@@ -397,6 +397,8 @@ export
    reim,
    reinterpret,
    rem,
+    rem1,
+    rem2pi,


I thought we removed rem1, and does rem2pi need to be exported?

🤷‍♂️

Honestly, I'm not sure. I'm also not sure why rem2pi takes a rounding argument but mod2pi doesn't.

mod2pi(x) is rem2pi(x, RoundDown)

StefanKarpinski · 2017-01-04T20:05:11Z

Ok, I resolved the conflicts as best I could, but to be perfectly honest, I have no idea what I'm doing here. Some are pretty clear (docstring added on master right below a new definition in this PR – keep both), others were way over my head – e.g. the previous modification to logdet in base/linalg/lu.jl. cc: @simonbyrne, @KristofferC, @andreasnoack, @jiahao, @sarvjeetsinghghotra.

tkelman · 2017-01-04T20:05:37Z

base/math.jl

-       airyai, airyaiprime, airybi, airybiprime,
-       airyaix, airyaiprimex, airybix, airybiprimex,
+       clamp, modf, ^, mod2pi, rem2pi,
+       airy, airyai, airyprime, airyaiprime, airybi, airybiprime, airyx,


wrong conflict resolution on the airy names I think

tkelman · 2017-01-04T20:05:51Z

base/math.jl

+rem2pi(x::Float32, ::RoundingMode{:Nearest}) = Float32(rem2pi(Float64(x),RoundNearest))
+rem2pi(x::Int32, ::RoundingMode{:Nearest}) = rem2pi(Float64(x),RoundNearest)
+function rem2pi(x::Int64, ::RoundingMode{:Nearest})
+  fx = Float64(x)


4 space indent

simonbyrne · 2017-01-05T15:59:08Z

Updated, but still needs some tests.

tkelman · 2017-01-05T16:03:24Z

base/math.jl

+  `[-abs(y)/2, abs(y)/2]`.
+
+- if `r == RoundToZero` (default), then the result is exact, and in the interval
+  `[0,abs(y))` if `x` is positive, or `(abs(y),0]` otherwise.


maybe math-mode (double backtick) the intervals since that isn't Julia notation?

simonbyrne · 2017-01-05T16:04:09Z

Two remaining questions:

should we deprecate mod2pi?
should we give rem2pi a default rounding mode?

tkelman · 2017-01-05T16:05:03Z

base/math.jl

+    x - 2π*round(x/(2π),r)
+
+without any intermediate rounding. This internally uses a high precision approximation of
+2π, and so will give a more `rem(x,2π,r)`


a more ... accurate result than ?

tkelman · 2017-01-05T16:05:35Z

base/math.jl

+  be the most accurate result.
+
+- if `r == RoundToZero`, then the result is in the interval `[0,2π]` if `x` is positive,.
+  or `[2π,0]` otherwise.


[-2π,0] ?

StefanKarpinski · 2017-01-05T18:23:07Z

Why is rem2pi preferable to mod2pi? Doesn't one need each in different situations? Shouldn't they both take rounding arguments and default to the current rounding mode?

simonbyrne · 2017-01-05T18:26:21Z

base/math.jl

+0.7853981633974481
+```
+"""
+mod2pi(x) = rem2pi(x,RoundDown)


@StefanKarpinski mod2pi is now just a special case of rem2pi

simonbyrne · 2017-01-09T15:38:50Z

I've added some tests. Comments welcome.

StefanKarpinski · 2017-01-11T05:47:17Z

Can't we just make rem2pi a method of mod2pi then?

simonbyrne · 2017-01-16T16:08:59Z

I think rem is more general that mod (mod to me implies that the values are always in the interval [0,x) for some x > 0).

tkelman · 2017-01-19T17:45:29Z

rebase then merge IMO?

tkelman · 2017-01-20T20:49:14Z

might be worth adding a news entry sometime before the release

StefanKarpinski · 2017-01-21T03:13:37Z

That is not correct: mod means that the value is in [0,m) for positive m or (m,0] for negative m, I.e. that the result sign always matches the modulus; rem means that the sign matches the argument rather than the modulus. For mathematical purposes, one generally wants mod not rem.

simonbyrne · 2017-01-21T08:51:55Z

I took rem to mean remainder after division, as described in #9283. In that case mod is a special case (being when the division is rounded down).

StefanKarpinski · 2017-01-23T21:31:08Z

@simonbyrne and I just had the following chat conversation which cleared this issue up for me.

Stefan Karpinski [2:40 PM]

@simonbyrne: I’m a bit unclear on what the difference between my understanding of rem and yours is

Simon Byrne [2:44 PM]

I'm not sure I understand what you're proposing
are you saying we should just make rem(x, y, r::RoundingMode), mod(x, y, r::RoundingMode)?

Stefan Karpinski [2:59 PM]

I guess I don’t understand why rem is more fundamental than mod in your definition of it
@simonbyrne ^
(sorry, trying to get this happening as a synchronous conversation)

Simon Byrne [3:00 PM]

to me, mod(x,y) implies that the result must be in the interval [0,y) (if y>0)

Stefan Karpinski [3:01 PM]

the more general definition is that mod(x,y) is in [0,y) for positive y and (y,0] for negative y

Simon Byrne [3:01 PM]

sure

Stefan Karpinski [3:01 PM]

which is usually what you actually want for a mathematical modulus operation

Simon Byrne [3:01 PM]

yes
but then what do you call the operation r = f(x,y) which gives -abs(y)/2 <= x <= abs(y)/2?
I tried to explain my thinking here:
#9283
The idea being that rem and mod are both the same sort of thing
finding r such that x = q*y + r for some integer q
and the difference is how you compute q
(or, really, the direction in which you round q)
and so "remainder" seems like the most general term for this operation
(similarly, we could also make fld and cld special cases of div(x,y,::RoundingMode))

Stefan Karpinski [3:06 PM]

Why make rem the fundamental one and not mod?

Simon Byrne [3:06 PM]

because you're finding a remainder
not a modulus

Stefan Karpinski [3:06 PM]

Can you explain how rounding is the difference between rem and mod?

Simon Byrne [3:08 PM]

so current rem is defined as
rem(x,y) = x - y*trunc(x/y) (edited)
(without intermediate rounding)
mod(x,y) = x - y*floor(x/y) (edited)
so #10946 defined
rem(x,y,r::RoundingMode) = x - y*round(x/y, r)
i.e. the remainder after Euclidean division with rounding direction r

Stefan Karpinski [3:11 PM]

I see
So you can get the operation you were talking about via a different rounding mode

Simon Byrne [3:12 PM]

yes, that would just be
rem(x,y, RoundNearest) = x - y*round(x/y)

Stefan Karpinski [3:13 PM]

standard rem, mod correspond to round towards zero (trump), and round down (floor), respectively

Simon Byrne [3:13 PM]

yes
exactly

Stefan Karpinski [3:13 PM]

Ok, I'm starting to get this now

Simon Byrne [3:14 PM]

my plan is also to define corresponding methods for div as well (edited)
since that is useful (e.g. for implementing fixed point arithmetic)

Stefan Karpinski [3:14 PM]

So the question becomes what function to make the three-argument form a method of

Simon Byrne [3:15 PM]

yes

Stefan Karpinski [3:15 PM]

A new function entirely, rem or mod

Simon Byrne [3:15 PM]

yes
personally, remainder seems like the best name for it

Stefan Karpinski [3:16 PM]

Is the issue with mod2pi that the behavior it implements does not correspond to what the mod function does?

Simon Byrne [3:17 PM]

no
our current mod2pi(x) = mod(x, 2pi)
which is okay
but not that useful

Stefan Karpinski [3:17 PM]

But with better precision

Simon Byrne [3:17 PM]

it's better to do the argument reduction to [-pi, pi], rather than [0, 2pi]

Stefan Karpinski [3:18 PM]

Ah, I see

Simon Byrne [3:18 PM]

e.g. if you get x = -0.000001

Stefan Karpinski [3:19 PM]

The trouble is that this operation has no standard name

Simon Byrne [3:19 PM]

and so that corresponds to rem(x, 2pi, RoundNearest)

Stefan Karpinski [3:19 PM]

Right, right

Simon Byrne [3:19 PM]

hence rem2pi(x, RoundNearest)
i mean we could punt on dispatch

Stefan Karpinski [3:19 PM]

?

Simon Byrne [3:19 PM]

define an twopi::Irrational
and rem(x, twopi, RoundNearest)

Stefan Karpinski [3:20 PM]

I think you mean tau

Simon Byrne [3:20 PM]

sure

Stefan Karpinski [3:21 PM]

I did add tau in a PR at one point, but I think I'd generally rather not start dispatching on irrationals too much

Simon Byrne [3:21 PM]

yeah

Stefan Karpinski [3:21 PM]

Gives people the wrong idea

Simon Byrne [3:21 PM]

it's not that useful
and could trigger a lot of unnecessary bikeshedding
best to leave that to Tau.jl

Stefan Karpinski [3:22 PM]

If pi works, why doesn't 2pi, and 3pi, and pi/2, etc
Sure

Simon Byrne [3:22 PM]

yeah
C calls rem(x,y,RoundNearest) remainder (edited)

Stefan Karpinski [3:23 PM]

I could actually imagine extending irrational to having integer and rational multiples

Simon Byrne [3:23 PM]

as does the IEEE spec

Stefan Karpinski [3:23 PM]

But that's way too big a change to drag into this

Simon Byrne [3:23 PM]

yeah
agreed
maybe for 2.0
or 3.14 if we're going for TeX numbering

Stefan Karpinski [3:24 PM]

Is that how rem is defined for floating point in C?

Simon Byrne [3:24 PM]

no rem and remainder are different

Stefan Karpinski [3:24 PM]

I.e. In a way that is incompatible with integer rem?
Oh, I see. It calls that operation remainder

Simon Byrne [3:25 PM]

C has fmod (which is rem(x::Float64, y::Float64)
and remainder (which is rem(x::Float64, y::Float64, RoundNearest))

Stefan Karpinski [3:26 PM]

I wonder if we should run this by a few people with good judgement on this sort of thing, e.g. @JeffBezanson and @stevengj

Simon Byrne [3:27 PM]

and remquo, which gives the same answer as remainder, but also the last few bits of the quotient via a pointer
it doesn't define a mod function

Stefan Karpinski [3:27 PM]

I can see rem being a good place for this but something feels oddly asymmetrical about it

Simon Byrne [3:27 PM]

which is why we had all the hassles defining it back in the day
(the day being 2013 or so)

Stefan Karpinski [3:28 PM]

Ok, so your plan is to make rem/div the fundamental pair with three-arg methods

Simon Byrne [3:28 PM]

yes

Stefan Karpinski [3:29 PM]

And then conceptually make all the two-arg versions aliases for calling those with various rounding modes

Simon Byrne [3:29 PM]

yes

Stefan Karpinski [3:29 PM]

What about rem2pi(x)
Which rounding more does that imply?

Simon Byrne [3:30 PM]

the same as rem(x, 2pi), i.e. RoundToZero

Stefan Karpinski [3:30 PM]

Sign trunc, i.e. Sign matches x?

Simon Byrne [3:30 PM]

yes

Stefan Karpinski [3:30 PM]

And round to nearest doesn't get a name

Simon Byrne [3:30 PM]

i mean, we could make it the default?
but I think people would find that confusing

Stefan Karpinski [3:31 PM]

Yes, that seems pretty confusing

Simon Byrne [3:31 PM]

also would break a lot

Stefan Karpinski [3:31 PM]

Ok, thanks for explaining this to me
You've sold me on your grand vision

…g#10946).

* Add NEWS.md entry for three-argument rem and the new rem2pi (#10946). * Clarify precise behaviour

* Add NEWS.md entry for three-argument rem and the new rem2pi (#10946). * Clarify precise behaviour (cherry picked from commit 8f9a948)

RoundNearest argument for rem

1b84e72

simonbyrne force-pushed the sb/rem-nearest branch from 4ce9424 to 1b84e72 Compare April 23, 2015 09:37

StefanKarpinski added the maths Mathematical functions label Sep 13, 2016

StefanKarpinski added this to the 0.6.0 milestone Sep 13, 2016

tkelman reviewed Jan 4, 2017

View reviewed changes

tkelman added needs docs Documentation for this change is required needs tests Unit tests are required for this change labels Jan 4, 2017

StefanKarpinski force-pushed the sb/rem-nearest branch from 885113e to 3f0ae5c Compare January 4, 2017 20:20

Merge branch 'master' into sb/rem-nearest

b0bf8b4

StefanKarpinski force-pushed the sb/rem-nearest branch from 3f0ae5c to b0bf8b4 Compare January 4, 2017 20:24

clean up rem/rem2pi functions

99fc8b8

tkelman reviewed Jan 5, 2017

View reviewed changes

tkelman removed the needs docs Documentation for this change is required label Jan 5, 2017

update docs based on comments

bc9ae48

simonbyrne commented Jan 5, 2017

View reviewed changes

Merge branch 'master' into sb/rem-nearest

9ac63f4

add tests, Float16, mpfr fix

86e78c8

tkelman removed the needs tests Unit tests are required for this change label Jan 9, 2017

Merge branch 'master' into sb/rem-nearest

a653e7f

simonbyrne merged commit cb1a47e into master Jan 20, 2017

simonbyrne deleted the sb/rem-nearest branch January 20, 2017 19:52

tkelman added the needs news A NEWS entry is required for this change label Jan 20, 2017

simonbyrne mentioned this pull request Jan 23, 2017

Euclidean division: div, rem and friends #9283

Closed

tkelman mentioned this pull request May 11, 2017

Make sure NEWS.md lists all 0.6 deprecations and breaking changes #21475

Closed

Sacha0 added a commit to Sacha0/julia that referenced this pull request May 11, 2017

Add NEWS.md entry for three-argument rem and the new rem2pi (JuliaLan…

7543e90

…g#10946).

Sacha0 mentioned this pull request May 11, 2017

add NEWS.md entry for three-argument rem and rem2pi #21812

Merged

tkelman removed the needs news A NEWS entry is required for this change label May 16, 2017

tkelman pushed a commit that referenced this pull request May 16, 2017

add NEWS.md entry for three-argument rem and rem2pi (#21812)

8f9a948

* Add NEWS.md entry for three-argument rem and the new rem2pi (#10946). * Clarify precise behaviour

tkelman pushed a commit that referenced this pull request May 17, 2017

add NEWS.md entry for three-argument rem and rem2pi (#21812)

5f9a9bb

* Add NEWS.md entry for three-argument rem and the new rem2pi (#10946). * Clarify precise behaviour (cherry picked from commit 8f9a948)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RFC: RoundNearest argument for rem #10946

RFC: RoundNearest argument for rem #10946

simonbyrne commented Apr 22, 2015

simonbyrne commented Apr 22, 2015

simonbyrne commented Apr 22, 2015

pao commented Apr 22, 2015

JeffBezanson commented Apr 25, 2015

StefanKarpinski commented Apr 25, 2015

StefanKarpinski commented Jan 4, 2017

tkelman Jan 4, 2017

StefanKarpinski Jan 4, 2017

StefanKarpinski Jan 4, 2017

simonbyrne Jan 5, 2017

StefanKarpinski commented Jan 4, 2017

tkelman Jan 4, 2017

tkelman Jan 4, 2017

simonbyrne commented Jan 5, 2017

tkelman Jan 5, 2017

simonbyrne Jan 5, 2017

simonbyrne commented Jan 5, 2017

tkelman Jan 5, 2017

tkelman Jan 5, 2017

StefanKarpinski commented Jan 5, 2017

simonbyrne Jan 5, 2017

simonbyrne commented Jan 9, 2017 •

edited

StefanKarpinski commented Jan 11, 2017

simonbyrne commented Jan 16, 2017

tkelman commented Jan 19, 2017

tkelman commented Jan 20, 2017

StefanKarpinski commented Jan 21, 2017

simonbyrne commented Jan 21, 2017

StefanKarpinski commented Jan 23, 2017

RFC: RoundNearest argument for rem #10946

RFC: RoundNearest argument for rem #10946

Conversation

simonbyrne commented Apr 22, 2015

simonbyrne commented Apr 22, 2015

simonbyrne commented Apr 22, 2015

pao commented Apr 22, 2015

JeffBezanson commented Apr 25, 2015

StefanKarpinski commented Apr 25, 2015

StefanKarpinski commented Jan 4, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

StefanKarpinski commented Jan 4, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simonbyrne commented Jan 5, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simonbyrne commented Jan 5, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

StefanKarpinski commented Jan 5, 2017

Choose a reason for hiding this comment

simonbyrne commented Jan 9, 2017 • edited

StefanKarpinski commented Jan 11, 2017

simonbyrne commented Jan 16, 2017

tkelman commented Jan 19, 2017

tkelman commented Jan 20, 2017

StefanKarpinski commented Jan 21, 2017

simonbyrne commented Jan 21, 2017

StefanKarpinski commented Jan 23, 2017

simonbyrne commented Jan 9, 2017 •

edited