GeometryTypes.jl icon indicating copy to clipboard operation
GeometryTypes.jl copied to clipboard

point arithmetic

Open ajkeller34 opened this issue 7 years ago • 5 comments

There are a few surprises (at least to me) that can happen with broadcasted Point arithmetic:

julia> [Point(1,1), Point(2,2), Point(3,3)] .- Point(1,-1)
ERROR: DimensionMismatch("arrays could not be broadcast to a common size")

julia> [Point(1,1), Point(2,2)] .- Point(1,-1)
2-element Array{Point{2,Int64},1}:
 [0, 0]
 [3, 3]

I would have instead expected:

julia> [Point(1,1), Point(2,2), Point(3,3)] .- Point(1,-1)
3-element Array{Point{2,Int64},1}:
 [0, 2]
 [1, 3]
 [2, 4]

julia> [Point(1,1), Point(2,2)] .- Point(1,-1)
2-element Array{Point{2,Int64},1}:
 [0, 2]
 [1, 3]

One possible fix on julia 0.7 is to define something like:

for f in (:+, :-)
    @eval Broadcast.broadcasted(::typeof($f), a::AbstractArray, p::Point) =
        Broadcast.broadcasted($f, a, StaticArrays.Scalar{typeof(p)}((p,)))
    @eval Broadcast.broadcasted(::typeof($f), p::Point, a::AbstractArray) =
        Broadcast.broadcasted($f, StaticArrays.Scalar{typeof(p)}((p,)), a)
end

Though I haven't thought through the broader implications, this is how I'm getting around the problem in a package I have that defines its own Point types. I'd much rather use the Point type defined here so that I can have compatibility with your visualization packages.

ajkeller34 avatar Aug 02 '18 22:08 ajkeller34

Point is an abstract vector, so this is kind of expected. I'd say if you want your point to be treated as a scalar in a given broadcasting operation, just wrap it in a Ref or tuple:

julia> [Point(1,1), Point(2,2), Point(3,3)] .- (Point(1, -1),)
3-element Array{Point{2,Int64},1}:
 [0, 2]
 [1, 3]
 [2, 4]

That seems pretty consistent with the v0.7 way of forcing users to be somewhat explicit about what should be a "scalar" in a given broadcasting operation.

rdeits avatar Aug 13 '18 23:08 rdeits

I see what you're saying, but what is more common: 1) given three points A, B, and P, wanting to add the x-coordinate of point P to both the x and y coordinates of point A, while also adding the y-coordinate of point P to the x and y coordinates of point B, or 2) wanting to add a point to a list of other points (i.e. translating a polygon)?

Does that first operation really make sense in the context of cartesian geometry? I guess I'm asking, would you ever want a point to broadcast as anything but a scalar? Perhaps I'm missing an obvious use case.

ajkeller34 avatar Aug 14 '18 00:08 ajkeller34

No, I think that's a reasonable question. I would venture that specifically overriding broadcasting for the case of Point + AbstractArray is going to be confusing for users, since it would result in Point + Set or Point + Generator returning something completely different than Point + AbstractArray. So perhaps the more central question is whether Point should always be scalar-like in broadcasting.

Perhaps you could try defining Base.broadcastable(p::Point) = Ref(p) ? That would make Point behave like a scalar in all broadcasting operations. Then we would have to think about what P .+ 1 would do. Currently it works (operating elementwise on P), but if Point broadcasts as a scalar then it will fail.

rdeits avatar Aug 14 '18 01:08 rdeits

I very much agree. Personally, I would gladly have P .+ 1 fail if it meant we could broadcast Point as a scalar. It doesn't seem like much of a loss to give up a shorthand way of translating a point in one specific direction.

ajkeller34 avatar Aug 15 '18 05:08 ajkeller34

I'm not sure if modifying this would help with other operations, like * and /? As of now I don't see a way of scaling an array of Points (or Vecs) by a Vec: [Point(1,1), Point(2,2), Point(3,3)] ./ (Vec(1, 1),) gives an array of 2x2 static matrices, and .* does not work at all.

aplavin avatar Oct 27 '18 16:10 aplavin