Make Ref behave as a 0-dimensional array in broadcast #18965

pabloferz · 2016-10-16T04:52:58Z

This implements the idea #18379 (comment) proposed by @stevengj of treating Refs as 0-dimensional arrays in broadcast.

yuyichao · 2016-10-16T05:11:08Z

Note that Ptr <: Ref.

stevengj · 2016-10-16T11:02:49Z

Use ::RefValue, not Ref?

stevengj · 2016-10-16T11:04:11Z

test/broadcast.jl

@@ -337,3 +337,9 @@ end
 @test broadcast(+, 1.0, (0, -2.0)) == (1.0,-1.0)
 @test broadcast(+, 1.0, (0, -2.0), [1]) == [2.0, 0.0]
 @test broadcast(*, ["Hello"], ", ", ["World"], "!") == ["Hello, World!"]
+
+# Issue #18379
+@test (+).(1, Ref(2)) == fill(3)


cute, I didn't realize that fill could be used to create a 0-dimensional array like this

pabloferz · 2016-10-16T13:27:34Z

Yeah, I guess what we need is just RefValue. If we are going to have this, should it make sense to have RefValue exported so people can access the constructor easily in case anyone want to wrap something?

vtjnash

I think the set Ref minus Ptr is a better API. The set RefArray + RefValue is rather arbitrary (it's incomplete).

vtjnash · 2016-10-16T13:45:27Z

base/broadcast.jl

@@ -127,6 +131,7 @@ dumpbitcache(Bc::Vector{UInt64}, bind::Int, C::Vector{Bool}) =
 Base.copy_to_bitarray_chunks!(Bc, ((bind - 1) << 6) + 1, C, 1, min(bitcache_size, (length(Bc)-bind+1) << 6))

 @inline _broadcast_getindex(A, I) = _broadcast_getindex(containertype(A), A, I)
+@inline _broadcast_getindex(A::RefArrOrVal, I) = A.x


@inline _broadcast_getindex(A::Ref, I) = A[]

I was thinking in limiting all this to RefValue. If we allow Ref and leave this definition of _broadcast_getindex as A[], then we will have

broadcast(+, [[0,2], [1,3]], Ref([1,-1])) == [[1,3], [2,4]]

where we want it to be [[1,1], [2,2]], which broadcast(+, [[0,2], [1,3]], RefValue([1,-1])) would correctly yield.

vtjnash · 2016-10-16T13:45:38Z

base/broadcast.jl

@@ -48,6 +51,7 @@ broadcast_indices(A) = broadcast_indices(containertype(A), A)
 broadcast_indices(::Type{Any}, A) = ()
 broadcast_indices(::Type{Tuple}, A) = (OneTo(length(A)),)
 broadcast_indices(::Type{Array}, A) = indices(A)
+broadcast_indices(::Type{Array}, A::RefArrOrVal) = ()


vtjnash · 2016-10-16T13:45:43Z

base/broadcast.jl

@@ -30,6 +32,7 @@ end
 containertype(x) = containertype(typeof(x))
 containertype(::Type) = Any
 containertype{T<:Tuple}(::Type{T}) = Tuple
+containertype{T<:RefArrOrVal}(::Type{T}) = Array


vtjnash · 2016-10-16T13:46:06Z

base/broadcast.jl

@@ -9,6 +9,8 @@ import Base: broadcast
 export broadcast!, bitbroadcast, dotview
 export broadcast_getindex, broadcast_setindex!

+typealias RefArrOrVal Union{Base.RefArray, Base.RefValue}


just use Ref

pabloferz · 2016-10-23T01:17:51Z

I updated the PR addressing @vtjnash comments and added news and docs.

In the docs there's now an example of how to use Ref to wrap a whole array (I didn't considered that at first so that's why I thought of using RefValue, but should be better now).

yuyichao · 2016-10-23T01:53:55Z

Seems that Ptr is still not handled.

pabloferz · 2016-10-23T14:30:20Z

Well, until we have something like #18990 (comment), I'd be inclined to throw an error for Ptr. Does that seem sensible?

yuyichao · 2016-10-23T15:04:45Z

I'd be inclined to throw an error for Ptr. Does that seem sensible?

No, because using Ptr as a normal (scalar) value in broadcast should be supported.

pabloferz · 2016-10-23T15:20:10Z

So Ref should behave as 0-dimensional array except for Ptr which should be treated as scalar?

EDIT: NVM. Considering what Ptr is, it is clear it should be treated as scalar.

yuyichao · 2016-10-23T15:25:03Z

I believe Ptr should be kept as scalar.

pabloferz · 2016-10-24T02:09:56Z

OK. Now it properly handles Ptr.

stevengj · 2016-10-24T17:44:58Z

LGTM.

tkelman · 2016-10-26T20:34:13Z

base/broadcast.jl

- - If there is at least an array in the arguments, it returns an array
- (and treats tuples as 1-dimensional arrays) expanding singleton dimensions.
+ - If there is at least an array or a `Ref` in the arguments, it returns an array
+ (and treats any `Ref` and tuple as 0-dimensional and 1-dimensional arrays,


Ref or tuple ?

"any Ref or tuple as a 0-dimensional or 1-dimensional array, respectively" would be correct, but it would perhaps be clearer to phrase it as "treats any Ref as a 0-dimensional array of its contents, and any tuple as a 1-dimensional array"

tkelman · 2016-12-15T05:02:31Z

base/broadcast.jl

- - If there is at least an array in the arguments, it returns an array
- (and treats tuples as 1-dimensional arrays) expanding singleton dimensions.
+ - If there is at least an array or a `Ref` in the arguments, it returns an array
+ (and treats any `Ref` as a 0-dimensional array of its contents and any tuple


missing close paren?

It is in the following line.

pabloferz · 2016-12-20T16:28:10Z

Rebased in top of the recent broadcast changes. Could this go into 0.6?

stevengj · 2016-12-20T18:09:40Z

LGTM

stevengj · 2016-12-23T17:01:06Z

Looks like we should define this for RefArray also. It is annoying to have to do Ref{Vector{Int}}([1,2,3]) rather than just Ref([1,2,3]).

yuyichao · 2016-12-23T17:06:03Z

RefArray is a reference of the element type, not the whole array.

stevengj · 2016-12-23T17:10:42Z

It would be nice to be able to do Ref(x) in a broadcast call and have x treated as a scalar. It is a lot less convenient if you have to do Ref{typeof(x)}(x).

RefArray contains all of the information that one needs (Ref([1,2,3]).x == [1,2,3]), so it is a bit frustrating.

yuyichao · 2016-12-23T17:56:49Z

This is not what Ref is meant to be used for. If we really want to use RefValue for this we should just export that.

pabloferz changed the title ~~Make Ref behave as a scalar wrapper for broadcast~~ Make Ref behave as a 0-dimensional array in broadcast Oct 16, 2016

yuyichao added the kind:breaking This change will break code label Oct 16, 2016

stevengj reviewed Oct 16, 2016

View reviewed changes

pabloferz changed the title ~~Make Ref behave as a 0-dimensional array in broadcast~~ Make RefValue behave as a 0-dimensional array in broadcast Oct 16, 2016

vtjnash reviewed Oct 16, 2016

View reviewed changes

pabloferz mentioned this pull request Oct 16, 2016

WIP: Allow constant fields in mutable types #11430

Closed

6 tasks

pabloferz force-pushed the pz/broadcast-ref branch from 7d120b9 to 09a9f03 Compare October 16, 2016 14:50

pabloferz mentioned this pull request Oct 17, 2016

Renaming Ref #18990

Closed

pabloferz force-pushed the pz/broadcast-ref branch 2 times, most recently from 246ff49 to 00ef35f Compare October 22, 2016 18:28

pabloferz changed the title ~~Make RefValue behave as a 0-dimensional array in broadcast~~ Make Ref behave as a 0-dimensional array in broadcast Oct 22, 2016

pabloferz force-pushed the pz/broadcast-ref branch from 00ef35f to bc247a4 Compare October 22, 2016 23:47

pabloferz added a commit to pabloferz/julia that referenced this pull request Oct 22, 2016

News and docs for JuliaLang#18965

bc247a4

pabloferz force-pushed the pz/broadcast-ref branch from bc247a4 to 8a4cb44 Compare October 23, 2016 20:19

pabloferz added a commit to pabloferz/julia that referenced this pull request Oct 23, 2016

News and docs for JuliaLang#18965

8a4cb44

tkelman reviewed Oct 26, 2016

View reviewed changes

pabloferz force-pushed the pz/broadcast-ref branch from 8a4cb44 to c930cfb Compare October 27, 2016 00:21

pabloferz added a commit to pabloferz/julia that referenced this pull request Oct 27, 2016

News and docs for JuliaLang#18965

c930cfb

pabloferz force-pushed the pz/broadcast-ref branch from c930cfb to 8e3f1d5 Compare November 4, 2016 15:21

pabloferz added a commit to pabloferz/julia that referenced this pull request Nov 4, 2016

News and docs for JuliaLang#18965

8e3f1d5

pabloferz closed this Nov 4, 2016

pabloferz reopened this Nov 4, 2016

pabloferz force-pushed the pz/broadcast-ref branch from 8e3f1d5 to bfa5477 Compare November 7, 2016 17:55

pabloferz added a commit to pabloferz/julia that referenced this pull request Nov 7, 2016

News and docs for JuliaLang#18965

bfa5477

pabloferz force-pushed the pz/broadcast-ref branch from bfa5477 to ffdbbcc Compare December 15, 2016 04:05

pabloferz added a commit to pabloferz/julia that referenced this pull request Dec 15, 2016

News and docs for JuliaLang#18965

ffdbbcc

tkelman reviewed Dec 15, 2016

View reviewed changes

pabloferz added 2 commits December 20, 2016 09:01

Make Ref behave as a scalar wrapper for broadcast

0351aa0

News and docs for JuliaLang#18965

986d575

pabloferz force-pushed the pz/broadcast-ref branch from ffdbbcc to 986d575 Compare December 20, 2016 15:03

stevengj merged commit 76a9180 into JuliaLang:master Dec 21, 2016

pabloferz deleted the pz/broadcast-ref branch December 21, 2016 15:25

pablosanjose mentioned this pull request Aug 13, 2017

Scalar type for broadcasting #18379

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make Ref behave as a 0-dimensional array in broadcast #18965

Make Ref behave as a 0-dimensional array in broadcast #18965

pabloferz commented Oct 16, 2016 •

edited

Loading

yuyichao commented Oct 16, 2016

stevengj commented Oct 16, 2016

stevengj Oct 16, 2016

pabloferz commented Oct 16, 2016

vtjnash left a comment

vtjnash Oct 16, 2016

pabloferz Oct 16, 2016

vtjnash Oct 16, 2016

vtjnash Oct 16, 2016

vtjnash Oct 16, 2016

pabloferz commented Oct 23, 2016 •

edited

Loading

yuyichao commented Oct 23, 2016

pabloferz commented Oct 23, 2016

yuyichao commented Oct 23, 2016 •

edited

Loading

pabloferz commented Oct 23, 2016 •

edited

Loading

yuyichao commented Oct 23, 2016

pabloferz commented Oct 24, 2016 •

edited

Loading

stevengj commented Oct 24, 2016

tkelman Oct 26, 2016

stevengj Oct 26, 2016

pabloferz Oct 27, 2016

tkelman Dec 15, 2016

pabloferz Dec 15, 2016

pabloferz commented Dec 20, 2016

stevengj commented Dec 20, 2016

stevengj commented Dec 23, 2016

yuyichao commented Dec 23, 2016

stevengj commented Dec 23, 2016

yuyichao commented Dec 23, 2016

Make Ref behave as a 0-dimensional array in broadcast #18965

Make Ref behave as a 0-dimensional array in broadcast #18965

Conversation

pabloferz commented Oct 16, 2016 • edited Loading

yuyichao commented Oct 16, 2016

stevengj commented Oct 16, 2016

Choose a reason for hiding this comment

pabloferz commented Oct 16, 2016

vtjnash left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pabloferz commented Oct 23, 2016 • edited Loading

yuyichao commented Oct 23, 2016

pabloferz commented Oct 23, 2016

yuyichao commented Oct 23, 2016 • edited Loading

pabloferz commented Oct 23, 2016 • edited Loading

yuyichao commented Oct 23, 2016

pabloferz commented Oct 24, 2016 • edited Loading

stevengj commented Oct 24, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pabloferz commented Dec 20, 2016

stevengj commented Dec 20, 2016

stevengj commented Dec 23, 2016

yuyichao commented Dec 23, 2016

stevengj commented Dec 23, 2016

yuyichao commented Dec 23, 2016

pabloferz commented Oct 16, 2016 •

edited

Loading

pabloferz commented Oct 23, 2016 •

edited

Loading

yuyichao commented Oct 23, 2016 •

edited

Loading

pabloferz commented Oct 23, 2016 •

edited

Loading

pabloferz commented Oct 24, 2016 •

edited

Loading