Add to Float arithmetic operators, is_finite,..., round, min/max. by Chris00 · Pull Request #1794 · ocaml/ocaml

Chris00 · 2018-05-23T09:06:25Z

Remark: The round function calls to C because the latter is usually implemented as ASM. Note that naive round functions do not work.

dbuenzli · 2018-05-23T09:38:48Z

Since this PR adds a wealth of predicates could you maybe also add the is_integer predicate ? It's useful in various conversion scenarios.

nojb · 2018-05-23T09:49:18Z

The AppVeyor failure is due to old versions of msvc not having round in their standard library. You will need to provide an implementation of round for them. @dra27 will probably know the full story about this.

Also, I think tests will be needed for the new functions.

Chris00 · 2018-05-23T10:07:48Z

old versions of msvc not having round in their standard library. You will need to provide an implementation of round for them

I saw... ☹ I thought that was something of the past...

I'll try to do the additional work ASAP.

Chris00 · 2018-05-24T10:44:49Z

@dbuenzli Wish granted! ☺

Chris00 · 2018-06-01T09:06:48Z

@mshinwell It seems Flambda transforms -0. in +0. through an incorrect optimization. Here is an example that does not use the new primitives (it is important that the condition of the first if be “opaque”):

let z =
  let x = -0. and y = +0. in
  if mod_float x 1. >= 0. then
    x
  else if false then x else y

let () =
  printf "%g\n" (1. /. z)

prints inf (it should be -inf).

xclerc · 2018-06-01T10:03:06Z

Here is a minimal reproduction case:

let f b =
  let x = -0. and y = +0. in
  if b then x else y

The problem is that flambda, when meeting the values of the
branches, uses = over float values. Thus considering that both
branches return the same value.

I will open a PR fixing the issue shortly.

xclerc · 2018-06-01T10:50:10Z

#1810 fixes the issue mentioned in the previous comments.

Chris00 · 2018-06-02T11:40:39Z

Rebased on trunk.

xclerc · 2018-06-04T09:40:49Z

+   when [x] or [y] is [nan].  Moreover [max (-0.) (+0.) = +0.] *)
+
+val minmax : float -> float -> float * float
+(** [minmax x y] returns [(x, y)] is [x <= y] and [(y, x)] otherwise.


"(...) returns (...) is (...)" should be "(...) returns (...) if (...)".

xclerc · 2018-06-04T09:42:54Z

+  = "caml_nextafter_float" "caml_nextafter" [@@unboxed] [@@noalloc]
+(** [nextafter x y] returns the next representable floating-point
+   value following [x] in the direction of [y].  If [y] is less than
+   [x], these functions will return the largest representable number


"these functions" seems to be from the C documentation...

It is – the documentation is heavily inspired from the C one.

I meant: shouldn't it be "this function"?

I modified it in the commit below.

xclerc · 2018-06-04T09:44:09Z


+external trunc : float -> float = "caml_trunc_float" "caml_trunc"
+                                    [@@unboxed] [@@noalloc]
+(** [trunc x] round [x] to the nearest integer whose absolute value is


"round" should be "rounds" for consistency.

xclerc · 2018-06-04T09:44:33Z

+(** [is_nan x] returns [true] if and only if [x] represents a [nan]. *)
+
+val is_integer : float -> bool
+(** Says whether the floating point is an integer. *)


"Says" should be "Say" for consistency.

xclerc · 2018-06-04T09:51:49Z

+(** [is_infinite x] says whether [x] is [infinity] or [neg_infinity]. *)
+
+val is_nan : float -> bool
+(** [is_nan x] returns [true] if and only if [x] represents a [nan]. *)


("[nan]" instead of "a [nan]"?)

I'm not sure what is the better (given that there are several NaNs, even though we do not distinguish them here).

I am not sure, but I think the documentation tends
to use "nan" rather than "a nan" in other places.

I can change it if you wish. Or maybe reformulate it as [x] is [nan].

I do not have a strong opinion on this question;
maybe @Octachron or @gasche will.

I like [x] is [nan]: it has the advantage of being correct when expanding it to [x] is not a number; moreover the Pervasives documentation already uses this wording.

xclerc · 2018-06-04T09:52:35Z

+   value following [x] in the direction of [y].  If [y] is less than
+   [x], these functions will return the largest representable number
+   less than [x].  If [x] equals [y], the function returns [y].
+   If [x] or [y] is a [nan], a [nan] is returned. *)


("[nan]" instead of "a [nan]"?)

Chris00 · 2018-06-21T09:13:46Z

Rebased.

dbuenzli

Thanks @Chris00 !

Chris00 · 2018-10-19T12:59:20Z

@gasche Fine with me.

xclerc · 2018-10-19T13:05:34Z

I have to say I have mixed feelings about pred/succ.

The introduction of one, zero, and minus_one to
be able to abstract over integer or float modules is fine,
but then the semantics of pred/succ is so different
that it is likely to cause more or less subtle problems.

xclerc · 2018-10-19T13:09:22Z

To be clear, I would prefer to have the functions defined as:

let pred x = x -. 1.0
let succ x = x +. 1.0

dbuenzli · 2018-10-19T13:10:10Z

but then the semantics of pred/succ is so different

It's not different it gives you the next value greater than the current one in the domain of the data type.

xclerc · 2018-10-19T13:13:27Z

I do understand your point of view, but I suspect some
code (arguably originally written with integers in mind)
would assume that succ x is x + 1.

dbuenzli · 2018-10-19T13:16:20Z

And maybe some code would simply like to get a value that is greater than the one that is given as argument. As far as I'm concerned pred and succ could also be defined on say type t = A | B | C.

xclerc · 2018-10-19T13:24:02Z

I would also be fine with pred/succ on type t = A | B | C
because this type maps to a subset of integers...

My point is that most of the existing code abstracting over
IntK.{pred,succ} expects the natural properties between
these functions and addition to hold. Passing Float will then
lead to unexpected results.

I just wanted to share a concern, you and the author of the
PR will decide which definitions should be used.

Chris00 · 2018-10-19T13:31:42Z

My point is that most of the existing code abstracting over IntK.{pred,succ}

Are there examples?

In the end, the question is what semantics we want to have for pred and succ. As aliases for x -> X.add x X.one and x -> X.sub x X.one, I am not sure they are interesting — and to me, the successor is the one that comes after, which depends on the type and does not necessarily correspond to “+1”.

xclerc · 2018-10-19T14:01:32Z

Are there examples?

To be fair, most of the examples I have in mind are so
closely related to integers that you would probably not
consider them. In Core, most occurrences of succ are
related to counters so one could reasonably move from
Int to Int{32,64} but not to Float.

In the end, the question is what semantics we want to have for pred and succ. As aliases for x -> X.add x X.one and x -> X.sub x X.one, I am not sure they are interesting — and to me, the successor is the one that comes after, which depends on the type and does not necessarily correspond to “+1”.

I won't say they are interesting as aliases, merely that they
are less surprising: replace Int32 with Float in a functor
application and you will approximately get the same results.

dbuenzli · 2018-10-19T14:16:51Z

I really prefer if we keep succ and pred as a convention for an ordered walk over the values of a type. As @Chris00 mentions I see little point of having that as an alias for add x one.

xclerc · 2018-10-19T18:31:46Z

Out of curiosity, is nan the predecessor/successor
of a non-nan value?

Chris00 · 2018-10-19T18:32:51Z

@xclerc no.

Chris00 · 2018-10-19T18:34:19Z

succ x is nan ⇔ x is nan

xclerc · 2018-10-19T18:41:08Z

@Chris00 thanks!

alainfrisch · 2018-10-23T11:21:24Z

Thanks @Chris00 for your efforts!

Chris00 force-pushed the float branch from ee65b0f to 07d1994 Compare May 23, 2018 10:06

alainfrisch added the stdlib label May 23, 2018

Chris00 force-pushed the float branch 5 times, most recently from a32d1d2 to 7b09918 Compare May 24, 2018 09:38

avsm reviewed May 24, 2018

View reviewed changes

Comment thread stdlib/float.ml

alainfrisch reviewed May 29, 2018

View reviewed changes

Comment thread stdlib/float.mli

alainfrisch reviewed May 29, 2018

View reviewed changes

Comment thread stdlib/float.ml

alainfrisch reviewed May 29, 2018

View reviewed changes

Comment thread stdlib/float.ml

Chris00 force-pushed the float branch from 7b09918 to 471bf73 Compare May 29, 2018 16:58

xclerc mentioned this pull request Jun 1, 2018

Use bit-pattern comparison when meeting float approximations (flambda) #1810

Merged

Chris00 force-pushed the float branch from 4e10c12 to 95ebccb Compare June 2, 2018 11:39

xclerc reviewed Jun 4, 2018

View reviewed changes

Chris00 force-pushed the float branch from d6480f4 to b87b241 Compare June 21, 2018 09:13

dbuenzli approved these changes Oct 19, 2018

View reviewed changes

Chris00 added 2 commits October 19, 2018 12:13

Add Float.min_max_num

ef1b877

Update Changes

2fda33f

Chris00 force-pushed the float branch from 86c5d58 to 2fda33f Compare October 19, 2018 10:14

alainfrisch reviewed Oct 19, 2018

View reviewed changes

Comment thread stdlib/float.mli

alainfrisch reviewed Oct 19, 2018

View reviewed changes

Comment thread testsuite/tests/lib-float/test.ml

Add some tests for pred and succ

7e3a04e

Chris00 force-pushed the float branch from e5038b8 to 7e3a04e Compare October 19, 2018 18:40

alainfrisch merged commit 00f9739 into ocaml:trunk Oct 23, 2018

Chris00 deleted the float branch October 23, 2018 23:08

damiendoligez pushed a commit to damiendoligez/ocaml that referenced this pull request Nov 5, 2018

Extend Stdlib.Float with more functions (ocaml#1794)

4d83754

vicuna mentioned this pull request Mar 14, 2019

OCaml lacks a proper implementation of min and max for floats #7892

Closed

dbuenzli mentioned this pull request Oct 30, 2019

Added some missing C99 float operations #944

Merged

xclerc mentioned this pull request Jul 30, 2020

Avoid polymorphic comparison over float values ocsigen/js_of_ocaml#1048

Merged

dbuenzli mentioned this pull request Apr 29, 2021

Inline and specialize Stdlib.{min,max} #10251

Closed

Conversation

Chris00 commented May 23, 2018

Uh oh!

dbuenzli commented May 23, 2018

Uh oh!

nojb commented May 23, 2018

Uh oh!

Chris00 commented May 23, 2018

Uh oh!

Chris00 commented May 24, 2018

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Chris00 commented Jun 1, 2018

Uh oh!

xclerc commented Jun 1, 2018

Uh oh!

xclerc commented Jun 1, 2018

Uh oh!

Chris00 commented Jun 2, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Chris00 commented Jun 21, 2018

Uh oh!

dbuenzli left a comment

Choose a reason for hiding this comment

Uh oh!

Chris00 commented Oct 19, 2018

Uh oh!

xclerc commented Oct 19, 2018

Uh oh!

xclerc commented Oct 19, 2018

Uh oh!

dbuenzli commented Oct 19, 2018

Uh oh!

xclerc commented Oct 19, 2018

Uh oh!

dbuenzli commented Oct 19, 2018

Uh oh!

xclerc commented Oct 19, 2018

Uh oh!

Chris00 commented Oct 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xclerc commented Oct 19, 2018

Uh oh!

dbuenzli commented Oct 19, 2018

Uh oh!

Uh oh!

Chris00 commented Oct 19, 2018 •

edited

Loading

Chris00 commented Oct 19, 2018 •

edited

Loading