JuliaLang / julia

The Julia Programming Language
https://julialang.org/
MIT License
45.74k stars 5.48k forks source link

v0.5 "cannot add methods to an abstract type" when overriding call #14919

Closed dlfivefifty closed 5 years ago

dlfivefifty commented 8 years ago

I'm updating ApproxFun and this caught me by surprise:

abstract Foo
julia> (f::Foo)(x)=x
ERROR: cannot add methods to an abstract type
 in eval(::Module, ::Any) at ./boot.jl:267

Is this intentional? Is there a way to get around this other than define for each subtype:

julia> immutable CFoo <: Foo end
julia> (f::CFoo)(x)=x
cortner commented 8 years ago

@vtjnash thank you - I only tested mine in a notebook mini-test. What would have gone wrong?

tkelman commented 8 years ago

If this feature gets implemented in a non-breaking way I guess it could be a backport candidate (as long as packages that rely on it are careful to mark the minimum patch version of Julia in their REQUIRE file)? Jeff, how likely would that be?

StefanKarpinski commented 8 years ago

Let's mark it as 0.5.x for now; if it's unlikely, we can bump it to 0.6

vtjnash commented 8 years ago

I wouldn't expect this to be non-breaking if implemented

StefanKarpinski commented 8 years ago

You never know :D

chipkent commented 8 years ago

Just noting that this change also causes me problems. Looks like I can work around it, but this should "just work".

vtjnash commented 8 years ago

Since this would be breaking to fix (because it adds a feature), moving this to the v0.6 target.

tkelman commented 8 years ago

if it's considered a regression from 0.4, then it would be fixing that. people would need to be careful about minimum version dependencies, and the fix would have to be non disruptive.

cortner commented 8 years ago

I think the current state is breaking while resolving this issue would keep compatibility

cortner commented 8 years ago

It is really too bad that this has moved to 0.6; it has really started to come back and bite me a few times since I first run into it.

JeffreySarnoff commented 8 years ago

I share that sentiment. Could we prioritize this within the stuff that is to be in v0.6 without detriment?

JeffreySarnoff commented 8 years ago

Do we yet know the syntax? I would rather move forward designing code with this facility and live with the fact that cannot be run and tested until tomorrow than require new stuff run in v0.5.

StefanKarpinski commented 7 years ago

This is unlikely to be fixed in 0.6.

JeffreySarnoff commented 7 years ago

@StefanKarpinski What are the two largest things in-the-way?

dpsanders commented 7 years ago

But hopefully will be for 1.0...

dlfivefifty commented 7 years ago

I agree with @dpsanders. This restriction is weird and feels artificial, which is fair enough in an 0.6 product, but could be damaging to Julia's reputation in a 1.0 product

dehann commented 7 years ago

I also just came across this and think it would be great if at least one of these cases could work (errors below):

abstract Foo <: Function
type Bar <: Foo
  var
end
# these 3 cases fail for different reasons
(mt::Foo)(x) = mt.var+x
(mt::T){T <: Foo}(x) = mt.var+x
# naively tried an unused syntax, which I would suggest as a syntax solution?
{T <: Foo}(mt::T)(x::Number) = mt.var+x

# but the more basic cases work, and are equal in this case
(mt::Bar){T <: Number}(x::T) = mt.var+x
(mt::Bar)(x::Number) = mt.var+x

# executing
bar = Bar(1.0)
@show bar(2.0)

This feels very close to polymorphism in Object Oriented, and should be possible. Problem is template syntax is not unique, since T could be <: Function or <: Number in the two cases above. However there is a bit of necessary redundancy in both basic cases.

cannot add methods to an abstract type
function type in method definition is not a type
syntax: invalid function name {T<:Foo}(mt:T)

# execution call is unique
MethodError: no method matching (::Bar)(::Float64)
# works from trivial case
bar(2.0) = 3.0

Related to @jiahao 's comment. cc @StefanKarpinski

martinholters commented 7 years ago

(::Function)(::Any...) = "I'm paving the road to hell."

dehann commented 7 years ago

Not sure I understand what you mean... I think the question is if type inference should be able to find abstract / templated functors. And as I understand, this used to be supported in 0.4. Conceptually from user perspective its not that different from the existing syntax for lambdas. For example this works:

(x...) -> @show x

I think the biggest design complication will be deciding on how to handle shared memory threads, but that is somewhat a separate issue. Dispatch and separate processes should all be accessing different memory locations, so doesn't seem a concern.

martinholters commented 7 years ago

If the above worked, then

julia> sin(1,2)
"I'm paving the road to hell."

I'm not saying this is necessarily bad, but it's certainly rather dangerous.

dehann commented 7 years ago

(::Foo)(::Any...) is clearly different and won't result in arbitrary calls. Plus this used to be supported...

swhalen commented 7 years ago

I've come across this while attempting to write a (small) computer algebra system to perform certain quantum mechanics calculations. Is this ever likely to be fixed, or should I perhaps consider rewriting in Python or similar?

StefanKarpinski commented 7 years ago

How is that the choice? I want this feature or I'm going to use a different language? This may happen at some point, but it's a fairly niche feature and not a high priority at the moment.

swhalen commented 7 years ago

The design I am looking to implement relies on inheriting call behavior from a supertype. In Python, to use my earlier example, one can override __call__ in an abstract class, and subclasses will inherit that behavior. It seems that, while this bug is outstanding, one can't do the same thing in Julia in an "easy" way (although it can probably be worked around with macros).

I understand of course that there is a need to prioritize features, and moreover that the developers are under no obligation to implement anything in particular just because a user wants it. I apologize if my earlier message came off as a demand.

cortner commented 7 years ago

I have to admit I also think of this as a bug rather than feature and I am surprised there aren't more Julia users complaining. But FWIW it is actually very easy to work around it with a macro.

swhalen commented 7 years ago

@cortner It seems to be easy to work around with a macro in the case where only a single such type hierarchy exists in the program -- indeed there's a macro doing exactly that earlier in this thread. If, however, I want several different abstract types, each of which has an associated "call behavior", I can't see a way to implement the desired behavior without some kind of lookup table. Any insights you may have into this problem would be much appreciated, because I am stumped!

cortner commented 7 years ago

EDIT: tested, it seems ok. One needs to work a bit more if there are type parameters. I can post this here if useful.

t_info(ex::Symbol) = (ex, tuple())
t_info(ex::Expr) = ex.head == :(<:) ? t_info(ex.args[1]) : (ex, ex.args[2:end])

macro ev(fsig)
   @assert fsig.head == :type
   tname, tparams = t_info(fsig.args[2])
   sym = esc(:x)
   return quote
       $(esc(fsig))      # creates the new type
       ($sym::$tname)(args...) = mycall($sym, args...)
   end
end

abstract A 
abstract B

@ev type AA <: A 
end 

@ev type BB <: B
end 

mycall(a::A) = "I am an A"
mycall(b::B) = "I am a B"

and then

julia> a = AA()
AA()

julia> a()
"I am an A"

julia> b = BB()
BB()

julia> b()
"I am a B"

P.S.: I use this construction a lot. Painful because I really don't like macros (took me ages + asking for help a lot), but it works for now.

swhalen commented 7 years ago

Thank you very much for the assistance, @cortner. I didn't think of using a generic function inside the macro -- I think that's quite clever.

If you could also post the version with type parameters it would be much appreciated.

cortner commented 7 years ago

https://github.com/libAtoms/JuLIP.jl/blob/864c3ba5e3b0ba4182f0fc09f65d1e66cc8e7a2b/src/potentials_base.jl#L47

swhalen commented 7 years ago

Thanks!

dlfivefifty commented 7 years ago

Personally I think it's a mistake to wait until 1.x to fix this bug: while it's not necessarily a crucial bug to fix, it's a confusing restriction that when hit, makes Julia feel "weird"

rakeshvar commented 6 years ago

+1 Need this feature to work for beauty and ease of coding (in my case).

marius311 commented 5 years ago

I haven't been able to find much discussion on this issue either here or on the forums since 1.0 came out. Are there any "new" workarounds for this with 1.0? (while awaiting a fix in 1.x it seems)

AzamatB commented 5 years ago

Bump. Just run into this.

cossio commented 5 years ago

Just got this error in Julia 1.1. Calling abstract types seems like a nice feature to have. Since there have been no comments here for a few months, what is the status of this issue? Will there be a fix eventually or will this remain an error in future versions?

StefanKarpinski commented 5 years ago

This is technically difficult and not a high priority, so I wouldn't hold your breath. Might happen eventually when there are no more important things on Jeff's plate or someone else takes it on.

findmyway commented 5 years ago

Could you share any plan (or hints) for how to implement this feature so that people who wish to have this feature may take some attempts?

(I was surprised to find this error for the first time and thought it should be relatively easy to support this feature. After struggling for a while, I had to admit that there was no easy approach.)

StefanKarpinski commented 5 years ago

@JeffBezanson may be able to provide some guidance.

JeffBezanson commented 5 years ago

This is certainly possible to implement. However, it is necessarily deeply tied to internals of how dispatch works, which is quite complex for performance reasons. So it may not be practical to attempt unless you really want to roll up your sleeves.

Currently (in what can perhaps be seen as a "premature optimization") the dispatch table is split based on the concrete type of the called object. Each TypeName has a mt::MethodTable field with the methods for just that type family (e.g. (f::F{T})() for any T). So a method lookup always starts with typeof(f).name.mt. Instead, we would need a single global MethodTable, and simply add all methods to it and do all lookups in it. I think that can be implemented fairly easily.

The main complication is that that is likely to cause some significant performance regressions in dynamic dispatch (see e.g. #21760 and linked issues), and also in method insertion (which is often the bottleneck in loading packages) and invalidation (the backedges array in MethodTable). Getting back this performance might be difficult, especially since it's not fast enough as it is.

JeffreySarnoff commented 5 years ago

for some purposes, this may suffice

abstract type Abstract end
isabstract(x) = false
isabstract(x::T) where {T<:Abstract} = true

abstract type Abstraction <: Abstract end
isabstraction(x) = false
isabstraction(x::T) where {T<:Abstraction} = true

struct Concrete <: Abstract
    value::String
end
concrete = Concrete("this is abstract")

struct Concretion <: Abstraction
    value::String
end
concretion = Concretion("this is abstraction")

(isabstract(concrete), isabstract(concretion)) == (true, true)
(isabstraction(concrete), isabstraction(concretion)) == (false, true)
JeffreySarnoff commented 5 years ago

@JeffBezanson is a global method table not akin the old "ball of call"?

AzamatB commented 5 years ago

Whoa! So excited to see this fixed! Thank you! :)