#33 integer? and round

closed-fixed
nobody
None
5
2011-08-30
2011-08-29
Doug Currie
No

r5rs requires that integer? return #t when (= x (round x))

r5rs requires round to preserve exactness

Now, round is an optional function (USE_MATH). I suggest that it should not be optional, rather that there be versions for USE_MATH and !USE_MATH; the !USE_MATH version could simply do

x=car(sc->args);
s_return(sc, ivalue(x)); /* truncate is not round, but is close enough for integer? to work */

In any case, once we have round, init.scm can contain:

; Utilities for math. Notice that inexact->exact is primitive,
; but exact->inexact is not.
(define exact? integer?)
;(define (inexact? x) (and (real? x) (not (integer? x))))
(define (inexact? x) (not (exact? x)))
; If x is an inexact real number, then (integer? x) is true if and only if (= x (round x))
(define round_ round)
(define (integer? x) (or (exact? x) (= x (round_ x))))
; round shoud return an exact if given an exact
(define (round x) (if (exact? x) x (round_ x)))
;

Discussion

  • Kevin Cozens

    Kevin Cozens - 2011-08-30

    Fix for integer? (to be tested)

     
  • Kevin Cozens

    Kevin Cozens - 2011-08-30
    • status: open --> pending
     
  • Kevin Cozens

    Kevin Cozens - 2011-08-30

    I have attached a file called integer.patch that deals with the integer? issue. It works but I'd appreciate independant testing to make sure there are no unwanted side effects caused by the patch.

     
  • Kevin Cozens

    Kevin Cozens - 2011-08-30

    Looking at the code again the is_integer() routine is only used in two places so I went ahead and commtted a modified version of the patch I attached along with a fix for round to preserve exactness. These fixes are in revision 82.

    I don't see the need to make round available when USE_MATH is not enabled. If you want to make a case for it, you can make your case for it on the tinyscheme mailing list.

     
  • Kevin Cozens

    Kevin Cozens - 2011-08-30
    • status: pending --> closed-fixed
     
  • Doug Currie

    Doug Currie - 2011-08-31

    It's really unfortunate that the name "integer" was used for the long integer values in tinyscheme. It makes the code confusing because the term is used differently in the scheme standards. It is really doubly confusing when there are …is_integer functions, some of which mean integer? and some mean exact? and some mean fixnum?

    You change seems dangerous to me. For example, some cases in the op_ switch statements may be depending on Eval_Cycle to confirm that the arguments are fixnums or positive fixnums (using is_integer or is_nonneg), and they could receive a float that happens to have no fractional part. In many cases this might work, but in the pre r82 code the cases in the op_ switch statements would be justified in using ivalue_unchecked which will now break.

    What I would do:
    1. rename your new is_integer to is_integerp and use it in OP_INTEGERP
    2. restore the old is_integer for use in is_nonneg and tests[]

    However, your is_integer function uses round_per_R5RS, but round_per_R5RS is only defined if USE_MATH -- this is what I was saying abut runs may need to be defined (or a substitute defined) if !USE_MATH.

    Now, an issue with OP_ROUND: You don't need to do any work if the argument's already an integer:

    case OP_ROUND:
    x=car(sc->args);
    if (num_is_integer(x))
    s_return(sc, x);
    else
    s_return(sc, mk_real(sc, round_per_R5RS(rvalue(x))));

     
  • Kevin Cozens

    Kevin Cozens - 2011-09-06

    Yes, it can seem a little confusing at times. It would be a little clearer if num_is_integer() was renamed num_is_exact(). An exact number is an integer while an inexact number may or may not be an integer. is_integer() will handle the case where an inexact number is an integer.

    is_integer() is only used by integer? and in is_nonneg(). My change to is_integer() called round_per_R5RS() which is only defined if USE_MATH is defined (as you rightly pointed out). Rounding a number to determine whether it is an integer is overkill. It is a lot simpler and faster to compare the ivalue and rvalue of the number.

    This reported was about integer? so I previously missed the comment about ROUND. It would be better to stick to one issue at a time in a bug report. Good point about ROUND when dealing with exact numbers. I committed the optimization for ROUND (in r87).

     

Log in to post a comment.