microsoft / QuantumLibraries

Q# libraries for the Quantum Development Kit
https://docs.microsoft.com/quantum
MIT License
543 stars 179 forks source link

Proposal: Math Lib is missing some APIs. #413

Closed kuzminrobin closed 3 years ago

kuzminrobin commented 3 years ago

(This is the original proposal text. See the updated version of this proposal below)

I propose (to the Q# API Design Review Board) to add to the Q# API the following missing calls to support the Math library.

    @Inline()
    function NAN() : Double {
        body intrinsic;
    }

    @Inline()
    function IsNan(d: Double) : Bool {
        body intrinsic;
    }

    @Inline()
    function INFINITY() : Double {
        body intrinsic;
    }

    @Inline()
    function IsInf(d: Double) : Bool {
        body intrinsic;
    }

    @Inline()
    function IsNegativeInfinity(d : Double) : Bool {
        body intrinsic;
    }

Currently they are needed to implement the Log() math function. More details: Discussion Possible Implementation

bamarsha commented 3 years ago

I think these don't have to be intrinsic, right?

function NaN() : Double {
    return 0.0 / 0.0;
}

function IsNaN(d : Double) : Bool {
    return d != d;
}

function Infinity() : Double {
    return 1.0 / 0.0;
}

function IsInfinity(d : Double) : Bool {
    // Or remove this function in favor of directly using "d == Infinity()".
    return d == Infinity();
}

function IsNegativeInfinity(d : Double) : Bool {
    // Or remove this function in favor of directly using "d == -Infinity()".
    return d == -Infinity();
}
cgranade commented 3 years ago

As a clarification, which namespace would you propose for these new callables? It seems like Microsoft.Quantum.Math may be the best bet, since these callables concern classical math operations similar to existing functions and operations in that namespace.

kuzminrobin commented 3 years ago

@samarsha, looks like you are right ;-) However I'm not sure if negative infinity is the same as -(positive infinity). It needs to be double-checked against the corresponding floating point standard.

kuzminrobin commented 3 years ago

@cgranade, I agree, Microsoft.Quantum.Math looks a good place.

bamarsha commented 3 years ago

I think you could define negative infinity as -1.0 / 0.0 in that case.

kuzminrobin commented 3 years ago

@samarsha, I like the ideas in both your of posts!

cgranade commented 3 years ago

I'd suggest going with IsInfinite for the name of the function to check against infinity and having it check against positive and negative infinity. In part, that makes for a nice complement with checking that a number is finite:

namespace Microsoft.Quantum.Math {
    function IsInfinite(d : Double) : Bool {
        return d == 1.0 / 0.0 or d == -1.0 / 0.0;
    }

    function IsFinite(d : Double) : Bool {
        return not IsInfinite(d) and not IsNaN(d);
    }
}

namespace Microsoft.Quantum.Diagnostics {
    function FiniteFact(d : Double, message : String) : Unit {
        Fact(IsFinite(d), message);
    }
}

Positive and negative infinities can still be discriminated by checking d > 0.0 and d < 0.0, so no expressive power is lost. This also more closely matches existing scientific computing APIs like np.isinf, 's isinf, or even MATLAB's isinf, all of which check against positive and negative infinity in precisely the same way.

cgranade commented 3 years ago

@kuzminrobin: The API review for March is coming up in a few days, it would be good to get this in to that review cycle. Would you be willing to update your proposal to include the namespace, and the feedback from me and @samarsh, so that we can get it added to the discussion? Thank you!

kuzminrobin commented 3 years ago

Based on the feedback/discussion, the updated version of the proposal is below:

namespace Microsoft.Quantum.Math {
    function NaN() : Double {
        return 0.0 / 0.0;
    }

    function IsNaN(d : Double) : Bool {
        return d != d;
    }

    function IsInfinite(d : Double) : Bool {
        return d == 1.0 / 0.0 or d == -1.0 / 0.0;
        // Positive and negative infinities can still be 
        // discriminated by checking `d > 0.0` and `d < 0.0`.
    }

    function IsFinite(d : Double) : Bool {
        return not IsInfinite(d) and not IsNaN(d);
    }
}

namespace Microsoft.Quantum.Diagnostics {
    function FiniteFact(d : Double, message : String) : Unit {
        Fact(IsFinite(d), message);
    }
}
cgranade commented 3 years ago

Based on https://github.com/microsoft/QuantumLibraries/pull/426, the proposal has been approved as written, thanks for driving this!

Based on the feedback/discussion, the updated version of the proposal is below:

namespace Microsoft.Quantum.Math {
    function NaN() : Double {
        return 0.0 / 0.0;
    }

    function IsNaN(d : Double) : Bool {
        return d != d;
    }

    function IsInfinite(d : Double) : Bool {
        return d == 1.0 / 0.0 or d == -1.0 / 0.0;
        // Positive and negative infinities can still be 
        // discriminated by checking `d > 0.0` and `d < 0.0`.
    }

    function IsFinite(d : Double) : Bool {
        return not IsInfinite(d) and not IsNaN(d);
    }
}

namespace Microsoft.Quantum.Diagnostics {
    function FiniteFact(d : Double, message : String) : Unit {
        Fact(IsFinite(d), message);
    }
}
kuzminrobin commented 3 years ago

Wow! Cool! What are the next steps for me? @bettinaheim, shall I make that change some time later? Shall I assign this to myself?

cgranade commented 3 years ago

@kuzminrobin: Similarly to #418, I'm happy either if you want to take this on, or if you'd prefer for me to. Either way, I've marked it in the March milestone to indicate that we'll try to take this on for the March release. As far as implementation, the main work here will be writing documentation comments and unit tests, since the implementation is already fully specified in the proposal itself.

cgranade commented 3 years ago

@bettinaheim @swernli: In terms of dependency ordering, should this one live in libraries or the runtime repo (i.e.: QSharpFoundation)?

swernli commented 3 years ago

This should go in the runtime repo, in QSharpFoundation. One thing I'm confused on:

function IsNaN(d : Double) : Bool {
    return d != d;
}

If I pass 4.0 to IsNaN won't it return true? Or is there something about doubles that I don't understand? I would've expected this to be:

function IsNaN(d : Double) : Bool {
    return d != NaN();
}
bamarsha commented 3 years ago

If I pass 4.0 to IsNaN won't it return true?

No, because 4.0 == 4.0.

function IsNaN(d : Double) : Bool {
    return d != NaN();
}

Do you mean d == NaN()? This doesn't work, since NaN is not equal to anything, even another NaN. Since it's the only floating point value with this property, a float is NaN iff it is not equal to itself (d != d).

swernli commented 3 years ago

Whoops, never mind. I inverted the Boolean in my head. If a double is NOT equal to itself then it is NaN. I get it now.

cgranade commented 3 years ago

This should go in the runtime repo, in QSharpFoundation. One thing I'm confused on:

function IsNaN(d : Double) : Bool {
    return d != d;
}

That's what I was thinking, but wasn't sure... I'll go on and add these there, then. Thanks!

If I pass 4.0 to IsNaN won't it return true?

No, because 4.0 == 4.0.

function IsNaN(d : Double) : Bool {
    return d != NaN();
}

Do you mean d == NaN()? This doesn't work, since NaN is not equal to anything, even another NaN. Since it's the only floating point value with this property, a float is NaN iff it is not equal to itself (d != d).

It will never sit well with me that IEEE 754 fails to be reflexive... I trip up all the time on NaN != NaN, sigh. All the more reason for good unit test coverage!

kuzminrobin commented 3 years ago

I'm happy either if you want to take this on, or if you'd prefer for me to

@cgranade, I can get back to this later. Feel free to go ahead and make the change if you have time and desire.

cgranade commented 3 years ago

@kuzminrobin: Since the March milestone will be closing in not too long, I'll go on and take care of this one, then. Want to make sure we don't miss the release window.

Grzegor232 commented 3 years ago

(This is the original proposal text. See the updated version of this proposal below)

I propose (to the Q# API Design Review Board) to add to the Q# API the following missing calls to support the Math library.

    @Inline()
    function NAN() : Double {
        body intrinsic;
    }

    @Inline()
    function IsNan(d: Double) : Bool {
        body intrinsic;
    }

    @Inline()
    function INFINITY() : Double {
        body intrinsic;
    }

    @Inline()
    function IsInf(d: Double) : Bool {
        body intrinsic;
    }

    @Inline()
    function IsNegativeInfinity(d : Double) : Bool {
        body intrinsic;
    }

Currently they are needed to implement the Log() math function. More details: Discussion Possible Implementation