Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unifier thinks 0.0 = -0.0 which means you can prove Void #2609

Closed
enolan opened this issue Sep 12, 2015 · 8 comments · Fixed by #3101
Closed

Unifier thinks 0.0 = -0.0 which means you can prove Void #2609

enolan opened this issue Sep 12, 2015 · 8 comments · Fixed by #3101

Comments

@enolan
Copy link
Contributor

enolan commented Sep 12, 2015

This isn't my discovery, this is copypasta from discussion on the IRC. Credit to {AS}, scshunt, Melvar and ladyfriday.

enolan at Behemoth in ~/mystuff/code/Idris-dev [master] 
$ .cabal-sandbox/bin/idris
     ____    __     _                                          
    /  _/___/ /____(_)____                                     
    / // __  / ___/ / ___/     Version 0.9.19-git:6fc713d
  _/ // /_/ / /  / (__  )      http://www.idris-lang.org/      
 /___/\__,_/_/  /_/____/       Type :? for help               

Idris is free software with ABSOLUTELY NO WARRANTY.            
For details type :warranty.
idris: bind: resource busy (Address already in use)
Idris> the ((False = True) -> Void) (\Refl impossible) $ cong {f = (>0) . (1/)} $ the (-0.0 = 0.0) Refl
case block in {val0} Void Refl Refl : Void

To explain, 1/-0.0 is -Infinity and 1/0.0 is +Infinity, so we have -Infinity = Infinity, False = True, and then Void. The unifier needs to be more precise about what floats are and aren't equal.

@mrmonday
Copy link
Contributor

This originally came about as a discussion of:

Idris> :let NaN = 0.0 / 0.0
Idris> the (NaN = NaN) Refl
(input):1:5:When elaborating argument x to function Prelude.Basics.the:
        Can't unify
                x = x
        with
                NaN = NaN

        Specifically:
                Can't unify
                        NaN
                with
                        NaN

Which itself came about in a discussion of how NaN behaves in types (it's interesting when Foo a unifies with Foo a, but Foo NaN doesn't with Foo NaN).

@ghost
Copy link

ghost commented Sep 12, 2015

I'll add that the fact that two NaNs won't unify is also potentially an issue, but we weren't able to find a way to get Void from that because, even though you can get NaN = NaN and NaN = NaN -> Void, the two won't unify.

@david-christiansen
Copy link
Contributor

The suggestion in the channel was bit-pattern identity for floats. This would make some NaNs equal, and others not, with no obvious way to tell the difference - I'm a bit worried about that. Also, does't IEEE mandate that NaN compared with anything is always False? On the other hand, worrying too much about the underlying meaning of machine types is perhaps the wrong thing to do, and bit-pattern equality is certainly easy to explain.

The proposal would also make 0.0 and -0.0 unequal, which is good.

@Melvar
Copy link
Collaborator

Melvar commented Sep 13, 2015

IEEE describes an equality test (and indeed a partial order) for floats which includes that negative zero is equal to positive zero, and that every NaN is incomparable with every NaN including itself. However it also describes a total order which will differentiate these cases, including different NaNs, and order them all, in such a way that x < y ⇒ totalOrder x y = LT but not the reverse. See IEEE floating point#Total-ordering predicate. Since it orders every possible float representation, the corresponding equality must be bit-pattern equality.

Which is to say, there is apparently a specific provision for this sort of requirement, and even a stronger one than ours. Now we just have to find implementations.

@ghost
Copy link

ghost commented Sep 13, 2015

Interesting!

My feeling about comparisons is that = is definitional equality, and that may differ from == equality. The issue with zeroes in particular means that we shouldn't be afraid to separate them for floating-point types.

The unification thing for NaNs means that writing patterns is hard, but in practice we would be better served by some sort of IsNan predicate or the like.

@Melvar
Copy link
Collaborator

Melvar commented Sep 13, 2015

Since there is no literal form for NaNs, you can’t match on them in the first place.

Edit: Wait, you mean IsNaN : Double -> Type?

@Melvar
Copy link
Collaborator

Melvar commented Sep 26, 2015

Allow me to add a possibly slightly prettier version of the proof at the beginning:

Idris> replace {P = \x => if 1 / x < 0 then () else Void} (the (-0.0 = 0.0) Refl) ()
() : Void

david-christiansen added a commit to david-christiansen/Idris-dev that referenced this issue Apr 8, 2016
Change the conversion checker to use bit-pattern identity on Double
instead of the IEEE comparison that Haskell's Eq type class implements.
Previously, we had definitionally equal values like -0.0 and 0.0 that
could nevertheless be distinguished by programs. This breaking of
functionality allowed proving a contradiction.

Thanks to Ryan Scott for the tip on how to do the bit-pattern
comparison!

Fixes idris-lang#2609.
@david-christiansen
Copy link
Contributor

I think that my concerns about bit-pattern identity for NaN were misplaced. We need to distinguish them for the same reason that we need to distinguish -0.0 and 0.0. #3101 does this.

david-christiansen added a commit that referenced this issue Apr 8, 2016
Change the conversion checker to use bit-pattern identity on Double
instead of the IEEE comparison that Haskell's Eq type class implements.
Previously, we had definitionally equal values like -0.0 and 0.0 that
could nevertheless be distinguished by programs. This breaking of
functionality allowed proving a contradiction.

Thanks to Ryan Scott for the tip on how to do the bit-pattern
comparison!

Fixes #2609.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants