Scryer-prolog: number_to_rational(0, R) should ideally be more efficient

Created on 1 May 2020  路  15Comments  路  Source: mthom/scryer-prolog

Currently, I get:

?- use_module(library(time)).
   true.
?- use_module(library(arithmetic)).
   true.
?- time(number_to_rational(0,R)).
   % CPU time: 37.599 seconds
   R = 0 rdiv 1

It is correct. However, ideally, it should finish sooner.

Also, it currently leaves a choicepoint:

;  % CPU time: 38.916 seconds
   false.

whereas ideally, number_to_rational/[2,3] are both deterministic.

enhancement

All 15 comments

As another test case, large integers should ideally also run very fast:

?- number_to_rational(500000, R).

This is because if N is an integer, then F is N rdiv 1 quickly yields a rational number that is arithmetically identical.

The case of 0 can't be handled this way, it should be handled as an exception.

Is float_integer_part, float_fractional_part part of the ISO?

For 0, I would expect 0 rdiv 1, for instance:

?- X is 0 rdiv 1.
   X = 0 rdiv 1.

You can test X against 0 with the arithmetic comparison X =:= 0.

And more generally, integer/1 can be used to test for integers.

If all integers are quickly converted to a rational number with X is N rdiv 1, then this would automatically also subsume the case N =:= 0.

It is unexpected but:
?- X is 1 rdiv 0.1, Y is floor(X), rational_numerator_denominator(X, N, D), R is N mod D, S is R / D, Q is N div D.
It isn't giving the right result, I expected Q to be 9.

I filed this as #435.

One thing that may help with this is specialized arithmetic instructions in the WAM, where the types of the operands are baked into the instruction, removing the need for case analysis in arithmetic functions. I have no idea how much of a speed boost it might provide, but it's worth a try. I meant to do that when I originally started compiling arithmetic instructions, but I held off on template specialization being added to Rust. It still hasn't been, unfortunately.

Maybe this can be used as an opportunity to motivate contributions to Rust itself, if you raise it at a suitable occasion in the Rust community?

I have a workaround but it's with //, still testing.

For a start, what do you think about the following version of number_to_rational/2, would that be an improvement:

number_to_rational(Real0, Fraction) :-
        (   var(Real0) -> instantiation_error(number_to_rational/2)
        ;   integer(Real0) -> Fraction is Real0 rdiv 1
        ;   rational(Real0) -> Fraction = Real0
        ;   float(Real0) -> number_to_rational(1.0e-6/1, Real0, Fraction)
        ;   domain_error(number, Real0, number_to_rational/2)
        ).

The key advantage is that for integers and rational numbers, the conversion is very easy and fast.

One related issue I find, with the current version:

?- number_to_rational(1/2, R).
   R = 1 rdiv 2.

This should instead raise a domain error, since 1/2 is not a number.

But there isn't a simplification ("good looking fraction") for rational but the integer is good.

Yes, that was before rational_numerator_denominator/3, it will be removed.

Yes, that's true! Then I think at least the case for integer/1 should be kept, and also the domain error in case the first argument is not a number.

This works nicely now, thank you very much!

(It is needed for library(simplex).)

Was this page helpful?
0 / 5 - 0 ratings

Related issues

notoria picture notoria  路  3Comments

triska picture triska  路  3Comments

notoria picture notoria  路  4Comments

triska picture triska  路  4Comments

Qqwy picture Qqwy  路  3Comments