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.

Sign up for GitHub

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

Jump to bottom

Make copy(DArray) copy #9745

Merged
merged 1 commit into from
Jan 13, 2015
Merged

Make copy(DArray) copy #9745

merged 1 commit into from
Jan 13, 2015

Conversation

andreasnoack
Copy link
Member

Right now copy(DArray) is a noop. I don't see why this is necessary and I'd like to be able to copy my DArrays when passing them to mutating linear algebra functions.

cc: @JeffBezanson

tkelman
tkelman reviewed Jan 12, 2015
View reviewed changes
base/darray.jl
@@ -245,7 +245,13 @@ end
getindex(d::DArray) = d[1]
getindex(d::DArray, I::Union(Int,UnitRange{Int})...) = sub(d,I...)

copy(d::SubOrDArray) = d
function copy!(dest::SubOrDArray, src::SubOrDArray)
dest.dims == src.dims && dest.pmap == src.pmap && dest.indexes == src.indexes && dest.cuts == src.cuts || throw(ArgumentError("desination array doens't fit to source array"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

destination array doesn't

@JeffBezanson
Copy link
Member

Having copy actually copy something? That's crazy! :)

andreasnoack added a commit that referenced this pull request Jan 13, 2015
@andreasnoack
Merge pull request #9745 from JuliaLang/anj/darraycopy
42e45a9 
Make copy(DArray) copy
@andreasnoack andreasnoack merged commit 42e45a9 into master Jan 13, 2015
@andreasnoack andreasnoack deleted the anj/darraycopy branch January 13, 2015 17:14
@tkelman
Copy link
Contributor

tkelman commented Jan 14, 2015

Turns out this breaks make testall1.

     * parallel
exception on 2: exception on ERROR: BoundsError: attempt to access 0x0 Array{Float64,2}
  at index [1]
 in getindex at array.jl:260
 in anonymous at multi.jl:1533
 in anonymous at multi.jl:844
 in run_work_thunk at multi.jl:603
 in run_work_thunk at multi.jl:612
 in anonymous at task.jl:6
2: ERROR: BoundsError: attempt to access 0x0 Array{Float64,2}
  at index [1]
 in getindex at array.jl:260
 in anonymous at multi.jl:1533
 in anonymous at multi.jl:844
 in run_work_thunk at multi.jl:603
 in run_work_thunk at multi.jl:612
 in anonymous at task.jl:6
exception on 1: ERROR: LoadError: test error in expression: fetch(@spawnat (workers())[1] (localpart(d))[1] != 0)
TypeError: non-boolean (BoundsError) used in boolean context
 in do_test at test.jl:48
 in runtests at /tmp/julia/share/julia/test/testdefs.jl:5
 in anonymous at multi.jl:642
 in run_work_thunk at multi.jl:603
 in remotecall_fetch at multi.jl:676
 in remotecall_fetch at multi.jl:691
 in anonymous at task.jl:1614
while loading parallel.jl, in expression starting on line 20
ERROR: LoadError: LoadError: test error in expression: fetch(@spawnat (workers())[1] (localpart(d))[1] != 0)
TypeError: non-boolean (BoundsError) used in boolean context
 in do_test at test.jl:48
 in runtests at /tmp/julia/share/julia/test/testdefs.jl:5
 in anonymous at multi.jl:642
 in run_work_thunk at multi.jl:603
 in remotecall_fetch at multi.jl:676
 in remotecall_fetch at multi.jl:691
 in anonymous at task.jl:1614
while loading parallel.jl, in expression starting on line 20
while loading /tmp/julia/share/julia/test/runtests.jl, in expression starting on line 42

@JeffBezanson
Copy link
Member

It's a behavior change, not a bug fix.

amitmurthy
amitmurthy reviewed Jan 14, 2015
View reviewed changes
test/parallel.jl

@test d == dc # Should be identical
@spawnat workers()[1] localpart(dc)[1] = 0
@test fetch(@spawnat workers()[1] localpart(d)[1] != 0) # but not point to the same memory
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be @spawnat id_other localpart(dc)[1] = 0 and @test fetch(@spawnat id_other localpart(d)[1] != 0) ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is probably true. I'll change it.

@andreasnoack
Copy link
Member Author

@tkelman I've pushed the change proposed by @amitmurthy so hopefully we won't see that test fail again.

@andreasnoack
Copy link
Member Author

I was actually awaiting the tests to finish, but now the commit should be there.

@tkelman
Copy link
Contributor

tkelman commented Jan 14, 2015

@andreasnoack sorry I'm impatient, make testall1 does work now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@andreasnoack @JeffBezanson @tkelman @amitmurthy