broken on julia 0.5

[mlubin]

Convex.jl will be broken on 0.5 until some thought is put into the concatenation behavior and fixing this. Now [1,2,3] lowers to Base.vect(1,2,3) instead of vcat(1,2,3). I spent a bit of time trying to figure out a quick fix but couldn't come up with anything.

[madeleineudell]

Miles, I think I understand what's going on now.

1. We can't overload hcat and vcat without clobbering the default methods from base, which we cleverly get around by rerouting them to cat(., 1) and cat(., 2). It's not great, but not terrible.

2. You're saying that there's no similar way to find a substitute for vect. So instead, you substituted vect by vcat, and that worked ok despite being ugly.

3. @andreasnoack and I worked on this problem yesterday. We tried and failed to find a nicer solution: we could do it if we could specify variable length arguments at the beginning of a call as well as at the tail - but the language doesn't support it. We could solve the problem by recursively breaking off the first argument until the first argument is an AbstractExpr, but would make these functions super slow (n recursive calls) when called on non-AbstractExprs, slowing down all of julia after loading Convex.

4. Just deleting the version check in your hack seemed to work for us, although it's still ugly, and reroutes vect to vcat. Do you know what the bad consequences of that are?

At this point, I'm just looking to minimize collateral damage in the way we fix this issue.

Madeleine

[mlubin]

Just deleting the version check in your hack seemed to work for us, although it's still ugly, and reroutes vect to vcat. Do you know what the bad consequences of that are?

Yes, it globally breaks julia:

julia> a = [1,2,3];

julia> [a,1]
2-element Array{Any,1}:
  [1,2,3]
 1       

julia> Base.vect(args...) = Base.vcat(args...)
WARNING: Method definition vect(Any...) in module Base at abstractarray.jl:24 overwritten in module Main at REPL[3]:1.

julia> [a,1]
4-element Array{Int64,1}:
 1
 2
 3
 1

[mlubin]

What about making vect a generated function? You could check if any of the arguments are an AbstractExpr and if not then fall back to the base definition.

[madeleineudell]

An excellent idea! The following seems to work (though I'm still getting an unrelated [I think] error):

@generated function vect(args...)
  for arg in args
    if arg<:AbstractExpr
      return :(HcatAtom([convert(AbstractExpr, arg)' for arg in args]...)')
    end
  end
  return :(vect(args...))
end

[madeleineudell]

Hm, no, not an unrelated error. The method isn't showing up correctly. Just to be safe, I've moved to the following function definition:

import Base.vect
export vect

@generated function Base.vect(args...)
  for arg in args
    if arg<:AbstractExpr
      return :(HcatAtom([convert(AbstractExpr, arg)' for arg in args]...)')
    end
  end
  # the following is the implementation of vect, copied from Base. 
  # can't figure out how not to overwrite it.
  # better would be to return :(Base.vect(args...))
  return :(T = Base.promote_typeof(args...); copy!(Array{T}(length(args)), args))
end

The error is

ERROR: LoadError: MethodError: no method matching isless(::Int64, ::Array{Convex.AbstractExpr,1})

For comparison, the following code works, and returns functions of both types. I'm not sure what the difference is.

type foo
end

@generated function yo(args...)
  for arg in args
    if arg<:foo
      return :(println("yo, foo"))
    end
  end
  return :(println("yo"))
end

yo(1,2,3)
yo(1,2,foo())
yo(1,2,foo(),4)
yo(1,2,3,4)

[madeleineudell]

@tkelman , do you have any ideas how we can overload vect in Convex without clobbering vect in Base? I'm getting nervous with v0.5 around the corner, and the issue is well above my pay grade.

[mlubin]

@madeleineudell, using your definition above, I get:

julia> using Convex
WARNING: ...

julia> Base.vect(1)
Error showing value of type Convex.TransposeAtom:
ERROR: MethodError: no method matching +(::Convex.Positive)
Closest candidates are:
  +(::Convex.Positive, ::Convex.Negative) at /home/mlubin/.julia/v0.5/Convex/src/dcp.jl:81
  +(::Convex.Positive, ::Convex.Positive) at /home/mlubin/.julia/v0.5/Convex/src/dcp.jl:79
  +(::Convex.Sign, ::Convex.NoSign) at /home/mlubin/.julia/v0.5/Convex/src/dcp.jl:85

Which makes very little sense. I think one needs to use Gallium here to figure out why a transpose atom is getting returned.

[yuyichao]

Instead of using a @generated function, it should be better to use a Base.@pure function to decide which one to use

function yo(args...)
    if need_hack(typeof(args))
      return println("yo, foo")
  end
  return println("yo")
end

There will still be method overwrite warnings though.

[madeleineudell]

Miles, the reason it's returning transpose atom is that the code is lazy:
right now we compute vcat (and so, also vect) by conjugating transpose with
hcat.

On Aug 8, 2016 3:14 PM, "Yichao Yu" notifications@github.com wrote:

Instead of using a @generated function, it should be better to use a
Base.@pure function to decide which one to use

function yo(args...)
if need_hack(typeof(args))
return println("yo, foo")
end
return println("yo")end

There will still be method overwrite warnings though.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#145 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAyp9F956Dh2vuICgb4TzSOB0C7HbP68ks5qdslegaJpZM4I-boq
.

[mlubin]

Fixed in #154 (I think)