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

Add proposed fluid syntax for select statement in Fluid's implementation of CSP #7908

Merged
merged 2 commits into from
Feb 5, 2018

Conversation

abhinavarora
Copy link
Contributor

No description provided.


select = fluid.select()

with sel.case(ch1, 'w', X):
Copy link
Contributor

@helinwang helinwang Jan 27, 2018

Choose a reason for hiding this comment

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

I think we need a way to differentiate the following two cases:

  select {
    case ch1 <- x:
      x := x + 1
    case y <- ch2:
      fmt.Println("Received on channel")
  }
  select {
    case ch1 <- x:
      x := x + 1
  }

  select {
    case y <- ch2:
      fmt.Println("Received on channel")
   }

The first one means wait for either of the case unblock, and continue. The second one means wait for the first unblock, and then wait for the second unblock. But I think the proposed Python syntax can't differentiate between the two case.

Choose a reason for hiding this comment

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

I agree with this as well, I think the default case in select should be the first example i.e. wait for either of the cases to unblock and if multiple cases unblock at the same time, we decide which block to execute pseudo-randomly. Otherwise execute the case that unblocked first.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry my mistake. There is a typo in the code. The code should be sel =fluid.select(). This way, the code can differentiate between the two cases. For the first case the code will be

sel1 = fluid.select()
with sel1.case(ch1, 'w', x):
    ...
    ...

with sel1.case(ch2, 'r', y):
    ...
    ...

For the second case the code will be like this

sel1 = fluid.select()
with sel1.case(ch1, 'w', x):
    ...
    ...

sel2 = fluid.select()
with sel2.case(ch2, 'r', y):
    ...
    ...


- `sel.case(ch1, 'w', X)` : This specifies that we are writing to `ch1` and we want to write the integer in variable `X` to the channel. The character `w` is used here to make the syntax familar to write syntax in Python I/O.

- `sel.case(ch2, 'r', Y)` : This specifies that we would like to read the result from `ch2` into variable `Y`. The character `r` is used here to make the syntax familar to read syntax in Python I/O.
Copy link
Contributor

Choose a reason for hiding this comment

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

An alternative to consider: sel.case(ch1.recv(x)) and sel.case(ch2.send(Y)).

Copy link

@kavyasrinet kavyasrinet Jan 27, 2018

Choose a reason for hiding this comment

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

My proposal is similar -- Defining the send and recv (write/read) for the Channel as @helinwang proposed above, or writing out send and recv as separate methods:

x = sel.case(fluid.recv(ch1))
sel.case(fluid.send(ch2, Y))

The methods can have the definition like:

void send(Channel* channel, Element* val)

Element* recv(Channel* channel)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree the C++ syntax should look like the one that you mentioned above. I am not sure how we would be able to implement sel.case(ch1.recv(x)) in python. This is because ch1.recv(x) can also be called in a standalone fashion, outside of select. I think this might depend on how we design the select operator in Paddle.

Copy link
Contributor

@helinwang helinwang Feb 5, 2018

Choose a reason for hiding this comment

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

Sorry for the late reply!

I have not find recv design in this doc, but I think ch1.recv(x) should return a var, so it could work both in standalone and in select.

Choose a reason for hiding this comment

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

I agree with @helinwang 's proposal as well.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree with @helinwang as well. I am approving this PR so we could merge it, together with @kavyasrinet 's PR on send/recv syntax. Then, let us rethink about them.


- `sel.case(ch1, 'w', X)` : This specifies that we are writing to `ch1` and we want to write the integer in variable `X` to the channel. The character `w` is used here to make the syntax familar to write syntax in Python I/O.

- `sel.case(ch2, 'r', Y)` : This specifies that we would like to read the result from `ch2` into variable `Y`. The character `r` is used here to make the syntax familar to read syntax in Python I/O.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree with @helinwang as well. I am approving this PR so we could merge it, together with @kavyasrinet 's PR on send/recv syntax. Then, let us rethink about them.

@wangkuiyi wangkuiyi merged commit 1ead6c2 into PaddlePaddle:develop Feb 5, 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.

4 participants