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.

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

Fix distribute function #72

Merged
merged 1 commit into from
Jul 7, 2016
Merged

Fix distribute function #72

merged 1 commit into from
Jul 7, 2016

Conversation

amitmurthy
Copy link
Contributor

This bug was causing the entire array to be distributed to be serialized to and from the workers in the init function.

@coveralls
Copy link

coveralls commented Jul 7, 2016

Coverage Status

Coverage decreased (-20.1%) to 50.0% when pulling c89fbce on amitm/expt into ec9ab98 on master.

@codecov-io
Copy link

codecov-io commented Jul 7, 2016

Current coverage is 69.01%

Merging #72 into master will decrease coverage by 1.10%

@@             master        #72   diff @@
==========================================
  Files             1          1          
  Lines           569        568     -1   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits            399        392     -7   
- Misses          170        176     +6   
  Partials          0          0          

Powered by Codecov. Last updated by ec9ab98...28707f5

@coveralls
Copy link

coveralls commented Jul 7, 2016

Coverage Status

Coverage decreased (-1.1%) to 69.014% when pulling 28707f5 on amitm/expt into ec9ab98 on master.

@amitmurthy amitmurthy merged commit 1a31742 into master Jul 7, 2016
@amitmurthy amitmurthy deleted the amitm/expt branch July 7, 2016 04:46
@andreasnoack
Copy link
Member

Ah. So there is actual a difference in the code between 0.4 and 0.5. I don't really get the difference here between the channel and future here. It would be great if you could elaborate. Thanks.

@amitmurthy
Copy link
Contributor Author

 owner = myid()
 rr = Future()
 put!(rr, A)
 d = DArray(size(A), procs, dist) do I
       remotecall_fetch(() -> fetch(rr)[I...], owner)
 end

Since serialization of "filled" Futures also includes the data of the Future, the previous code (shown above) was resulting in the entire array A, first being sent as part of the closure (due to rr being captured in the DArray init function) to each worker, then from each worker back to the master (remotecall_fetch on the owner) and finally the requested sub-array was being returned to the worker.

Changing it to RemoteChannel, which always only sends 3 integers ( where, whence and id ) fixed it.

In 0.4, this was a RemoteRef (which behaves like a RemoteChannel) and hence was not an issue. The bug crept in during the 0.5 port.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants