Skip to content

Conversation

@abhinavarora
Copy link
Contributor

This PR also contains a notest_concuurency.py that is a test that we need to pass when go op and channels are implemented in C++

@cs2be cs2be changed the title [WIP]Python implementation for a proposed Go Op. Python implementation for a proposed Go Op. Feb 13, 2018
@cs2be cs2be mentioned this pull request Feb 13, 2018
from core import LoDTensor, CPUPlace, CUDAPlace
from distribute_transpiler import DistributeTranspiler
from distribute_transpiler_simple import SimpleDistributeTranspiler
from concurrency import *
Copy link
Collaborator

Choose a reason for hiding this comment

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

According to the PEP8 style:

Wildcard imports (from import *) should be avoided, as they make it unclear which names are present in the namespace, confusing both readers and many automated tools.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for pointing that out. Fixed in the latest commit.

]


class Go(BlockGuard):
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't get it why would we need to define Go as a class?

In https://github.com/PaddlePaddle/Paddle/tree/develop/python/paddle/v2/fluid, I don't see a While class or an IfThenElse class.

Copy link

@kavyasrinet kavyasrinet Feb 13, 2018

Choose a reason for hiding this comment

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

Oh the While and IfElse classes are in https://github.com/PaddlePaddle/Paddle/blob/develop/python/paddle/v2/fluid/layers/control_flow.py . We thought we'll put all CSP related operators in concurrency.py and they need not be a part of layers as while and ifelse are so we put it right under the fluid directory.

@kavyasrinet kavyasrinet merged commit 74404fa into PaddlePaddle:develop Feb 16, 2018
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.

5 participants