Skip to content
This repository has been archived by the owner on Jan 12, 2024. It is now read-only.

Proposal: Math Lib is missing some APIs. #413

Closed
kuzminrobin opened this issue Feb 8, 2021 · 20 comments
Closed

Proposal: Math Lib is missing some APIs. #413

kuzminrobin opened this issue Feb 8, 2021 · 20 comments
Assignees
Labels
Area-API Issue concerns the API design of a library, such as style guide or design principles adherence. Kind-Enhancement New feature or request Pkg-Standard Issue relates to the Microsoft.Quantum.Standard package. Resolution-Done Issue closed because work item completed.
Milestone

Comments

@kuzminrobin
Copy link
Contributor

kuzminrobin commented Feb 8, 2021

(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
Copy link
Contributor

bamarsha commented Feb 8, 2021

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 cgranade added Area-API Issue concerns the API design of a library, such as style guide or design principles adherence. Kind-Enhancement New feature or request Pkg-Standard Issue relates to the Microsoft.Quantum.Standard package. Status-NeedsApiReview This PR requires an API review before merging in. labels Feb 8, 2021
@cgranade
Copy link
Contributor

cgranade commented Feb 8, 2021

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
Copy link
Contributor Author

@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
Copy link
Contributor Author

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

@bamarsha
Copy link
Contributor

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

@kuzminrobin
Copy link
Contributor Author

kuzminrobin commented Feb 17, 2021

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

@cgranade
Copy link
Contributor

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
Copy link
Contributor

cgranade commented Mar 1, 2021

@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
Copy link
Contributor Author

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 cgranade removed the Status-NeedsApiReview This PR requires an API review before merging in. label Mar 9, 2021
@cgranade
Copy link
Contributor

cgranade commented Mar 9, 2021

Based on #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
Copy link
Contributor Author

kuzminrobin commented Mar 9, 2021

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 cgranade added this to the March 2021 milestone Mar 10, 2021
@cgranade
Copy link
Contributor

@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
Copy link
Contributor

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

@swernli
Copy link
Contributor

swernli commented Mar 11, 2021

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
Copy link
Contributor

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
Copy link
Contributor

swernli commented Mar 11, 2021

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
Copy link
Contributor

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
Copy link
Contributor Author

kuzminrobin commented Mar 11, 2021

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
Copy link
Contributor

@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
Copy link

(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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Area-API Issue concerns the API design of a library, such as style guide or design principles adherence. Kind-Enhancement New feature or request Pkg-Standard Issue relates to the Microsoft.Quantum.Standard package. Resolution-Done Issue closed because work item completed.
Projects
None yet
Development

No branches or pull requests

5 participants