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

better fallback ::Integer methods for flipsign and cmp #24437

Open
JeffreySarnoff opened this issue Nov 2, 2017 · 16 comments
Open

better fallback ::Integer methods for flipsign and cmp #24437

JeffreySarnoff opened this issue Nov 2, 2017 · 16 comments

Comments

@JeffreySarnoff
Copy link
Contributor

(It happened yesterday, a holdover from v0.2)

while trying approaches to keeping SafeInt types stickier than Ints
(a) just follow JuliaMath/IntegerRounding
(b) (a) with specific conversions xor constructions
(c) (a) with specific conversions and constructions
at some point, these errors occur, each as the prior is fixed

flipsign not defined for SafeInt64
( flipsign(SafeInt64, Int64), -SafeInt64, SafeInt<SafeInt were defined )

flipsign(flipee:SafeInt64, flipper::Int64)
could fallback to flipper < 0 ? -flippee : flippee
if that evaluates unexceptionally, the proper result is obtained, right?

< not defined for SafeInt64, Int64
( the conversions and promotions were defined )

cmp(x,y) could fallback to (cmp)(promote(x,y)...)
if that evaluates unexceptionally, the proper result is obtained, right?

dec not defined for ?

@KristofferC
Copy link
Sponsor Member

KristofferC commented Nov 2, 2017

Perhaps it is because I am not a native speaker, but I am having quite a hard time understanding what the problem is. Perhaps you make a minimum example, with the expected output and the current output?

(also, please use code quotes)

@JeffreySarnoff
Copy link
Contributor Author

I do not have access to the history now. Maybe Stefan remembers what makes the 'flipsign' error appear. Otherwise, when I am at that machine I will try to isolate this.

@JeffreySarnoff
Copy link
Contributor Author

JeffreySarnoff commented Nov 2, 2017

@StefanKarpinski remember this -- the fallback first to a flipsign error and then to a dec error when defining a new low level arithmetic type?

@StefanKarpinski
Copy link
Sponsor Member

I'm afraid I don't recall this. When did we discuss it?

@JeffreySarnoff
Copy link
Contributor Author

2013

@JeffreySarnoff
Copy link
Contributor Author

I will try to re-create it tonight

@stevengj
Copy link
Member

stevengj commented Nov 2, 2017

@JeffreySarnoff, we already have a fallback method for flipsign, as well as more specific flipsign fallbacks for integer types. Why aren't these working for you? Which method of flipsign is being called?

Is SafeInt64 a subtype of Signed?

@JeffreySarnoff
Copy link
Contributor Author

Is SafeInt64 a subtype of Signed? yes

@JeffreySarnoff
Copy link
Contributor Author

v0.7.0-DEV

sizeof(Int) == 8

abstract type SafeSigned <: Signed end

primitive type SafeInt64 <: SafeSigned 64 end

Base.convert(::Type{SafeInt64}, x::Int64)     = reinterpret(SafeInt64, x)
Base.convert(::Type{Int64},     x::SafeInt64) = reinterpret(Int64, x)

s64 = SafeInt64(3)

Error showing value of type SafeInt64:
ERROR: flipsign not defined for SafeInt64

@KristofferC
Copy link
Sponsor Member

0.6

julia> s64 = SafeInt64(3)
Error showing value of type SafeInt64:
ERROR: circular method definition: promotion of types SafeInt64 and SafeInt64 failed to change any input types
Stacktrace:
 [1] error(::String, ::String, ::Vararg{String,N} where N) at /Applications/Julia-0.6.app/Contents/Resources/julia/lib/julia/sys.dylib:?
 [2] sametype_error(::SafeInt64, ::Vararg{SafeInt64,N} where N) at ./promotion.jl:244
 [3] promote_noncircular at ./promotion.jl:219 [inlined]
 [4] flipsign at ./int.jl:75 [inlined]
 [5] abs at ./int.jl:112 [inlined]
 [6] dec(::SafeInt64) at ./intfuncs.jl:492
 [7] show at ./show.jl:259 [inlined]
 [8] show at ./replutil.jl:4 [inlined]

@JeffreySarnoff
Copy link
Contributor Author

0.7-DEV

julia> s64 = SafeInt64(3)
Error showing value of type SafeInt64:
ERROR: flipsign not defined for SafeInt64
Stacktrace:
 [1] error(::String, ::String, ::Type) at ./error.jl:44
 [2] dec(::SafeInt64) at ./intfuncs.jl:654
 [3] show(::IOContext{Base.Terminals.TTYTerminal}, ::SafeInt64) at ./show.jl:353
 [4] show(::IOContext{Base.Terminals.TTYTerminal}, ::MIME{Symbol("text/plain")}, ::SafeInt64) at ./replutil.jl:4
 [5] display(::Base.REPL.REPLDisplay{Base.REPL.LineEditREPL}, ::MIME{Symbol("text/plain")}, ::SafeInt64) at ./repl/REPL.jl:125
 [6] display(::Base.REPL.REPLDisplay{Base.REPL.LineEditREPL}, ::SafeInt64) at ./repl/REPL.jl:128
 [7] display(::SafeInt64) at ./multimedia.jl:277
 [8] (::getfield(Base, Symbol("#inner#4")){Array{Any,1},typeof(display),Tuple{SafeInt64}})() at ./essentials.jl:669
 [9] print_response(::Base.Terminals.TTYTerminal, ::Any, ::Void, ::Bool, ::Bool, ::Void) at ./repl/REPL.jl:146
 [10] print_response(::Base.REPL.LineEditREPL, ::Any, ::Void, ::Bool, ::Bool) at ./repl/REPL.jl:132
 [11] (::getfield(Base.REPL, Symbol("#do_respond#17")){Bool,getfield(Base.REPL, Symbol("##27#37")){Base.REPL.LineEditREPL,Base.REPL.REPLHistoryProvider},Base.REPL.LineEditREPL,Base.LineEdit.Prompt})(::Base.LineEdit.MIState, ::Base.GenericIOBuffer{Array{UInt8,1}}, ::Bool) at ./repl/REPL.jl:674
 [12] (::getfield(Base.REPL, Symbol("##32#42")))(::Base.LineEdit.MIState, ::Base.REPL.LineEditREPL, ::Vararg{Any,N} where N) at ./repl/REPL.jl:929
 [13] (::getfield(Base.LineEdit, Symbol("##19#20")){getfield(Base.REPL, Symbol("##32#42")),String})(::Base.LineEdit.MIState, ::Base.REPL.LineEditREPL) at ./repl/LineEdit.jl:1059
 [14] prompt!(::Base.Terminals.TTYTerminal, ::Base.LineEdit.ModalInterface, ::Base.LineEdit.MIState) at ./repl/LineEdit.jl:2061
 [15] run_interface(::Base.Terminals.TTYTerminal, ::Base.LineEdit.ModalInterface) at ./repl/LineEdit.jl:1962
 [16] run_frontend(::Base.REPL.LineEditREPL, ::Base.REPL.REPLBackendRef) at ./repl/REPL.jl:986
 [17] run_repl(::Base.REPL.LineEditREPL, ::getfield(Base, Symbol("##566#567"))) at ./repl/REPL.jl:182
 [18] _start() at ./client.jl:448

@KristofferC
Copy link
Sponsor Member

flipsign(x::T, y::T) where {T<:Signed} = no_op_err("flipsign", T)

@stevengj
Copy link
Member

stevengj commented Nov 2, 2017

That's weird, I wonder why we have that fallback, given that we have a generic flipsign(::Real, ::Real) fallback. Is it to prevent a dispatch loop somewhere?

@KristofferC
Copy link
Sponsor Member

KristofferC commented Nov 2, 2017

Yes, #23491, #22801 (related to the error on 0.6)

@stevengj
Copy link
Member

stevengj commented Nov 2, 2017

Seems like the fix should be to have the fallback call the ::Real version (either by invoke, if that is fast nowadays, or by refactoring have them both call _flipsign or something).

@stevengj stevengj changed the title inelegant fallback better fallback ::Signed methods for flipsign and cmp Nov 2, 2017
@JeffreySarnoff
Copy link
Contributor Author

and better fallback ::Unsigned methods too .. things work better when each (Signed, Unsigned) is separately backstopped (otherwise Bool <: Integer can make less obvious demands on the type writer)

@stevengj stevengj changed the title better fallback ::Signed methods for flipsign and cmp better fallback ::Integer methods for flipsign and cmp Nov 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants