search

mila-iqia / blocks-extras

A collection of extensions to the Blocks framework
MIT License
27 stars 40 forks source link

"Fixed" Platoon integration #44

Closed mgermain closed 8 years ago

mgermain commented 8 years ago

It was already working with the last version but I removed some unnecessary code for operations that are now done seamlessly directly in Platoon.

nouiz commented 8 years ago

blocks have its own dispatcher of process. Should we remove it? Can you compare if the one in platoon cover all what the one here do?

rizar commented 8 years ago

@nouiz , your comment leaves me puzzled. All we have in Blocks is a script to run the controller, and it is assumed that the workers are launched manually.

nouiz commented 8 years ago

I taught that script was also starting the worker. If you think it is still needed, we can keep it, but if we don't need it, I think we should clean this up now in this PR if possible.

On Thu, Feb 18, 2016 at 1:13 PM, Dzmitry Bahdanau notifications@github.com wrote:

@nouiz https://github.com/nouiz , your comment leaves me puzzled. All we have in Blocks is a script to run the controller, and it is assumed that the workers are launched manually.

— Reply to this email directly or view it on GitHub https://github.com/mila-udem/blocks-extras/pull/44#issuecomment-185841820 .

mgermain commented 8 years ago

No, they do need the controller.

On Thu, Feb 18, 2016 at 7:59 PM, Frédéric Bastien notifications@github.com wrote:

I taught that script was also starting the worker. If you think it is still needed, we can keep it, but if we don't need it, I think we should clean this up now in this PR if possible.

On Thu, Feb 18, 2016 at 1:13 PM, Dzmitry Bahdanau < notifications@github.com> wrote:

@nouiz https://github.com/nouiz , your comment leaves me puzzled. All we have in Blocks is a script to run the controller, and it is assumed that the workers are launched manually.

— Reply to this email directly or view it on GitHub < https://github.com/mila-udem/blocks-extras/pull/44#issuecomment-185841820>

.

— Reply to this email directly or view it on GitHub https://github.com/mila-udem/blocks-extras/pull/44#issuecomment-185999201 .

rizar commented 8 years ago

Should I merge this, or do you think that something else should be done here?

mgermain commented 8 years ago

Looks good to me.

On Fri, Feb 19, 2016 at 10:34 AM, Dzmitry Bahdanau <notifications@github.com

wrote:

Should I merge this, or do you think that something else should be done here?

— Reply to this email directly or view it on GitHub https://github.com/mila-udem/blocks-extras/pull/44#issuecomment-186262018 .