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

Create select_op design document #9139

Merged
merged 5 commits into from
Mar 20, 2018

Conversation

cs2be
Copy link
Contributor

@cs2be cs2be commented Mar 15, 2018

No description provided.

@@ -0,0 +1,94 @@
# select_op Design

Choose a reason for hiding this comment

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

Commenting on the graphics here:

  • In select_op.png, the cases attributes does not make sense if you don't explicitly annotate what those comma-separated values represent
  • In Block0, some indication of the fact that there are other blocks before and after the select op
  • The Block1 diagram is difficult to understand, even though you have all the right things in all the right places. Not that this fixes the problem, but the fill_contants being used with different values 0 and 1 doesn't make much sense - it needs to be explicit about its use of index.

Choose a reason for hiding this comment

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

In select_op_workflow: The "push queuemessage on all involved channels and block current thread" is a very loaded concept. Either this gets broken down here or in text or really well picked up and talked about in the channels doc.

For the line that says "execute sub_block", it would be much nicer if the previous diagram has a big bold name on that block, and you use the same name here. Or use colors. Or something that makes the connection between the two different places where the "sub block" is being talked about.

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 can change the select_op.png to just show the actual program desc. I'lll also update the text for push queuemessage


## Introduction

In golang, the **select** statement lets a goroutine wait on multiple

Choose a reason for hiding this comment

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

Wouldn't hurt to have a link to the go tutorial page

## Introduction

In golang, the **select** statement lets a goroutine wait on multiple
communication operations at the same time. The **select** blocks until

Choose a reason for hiding this comment

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

channel operations. Being agnostic here does not have much of a purpose here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Select depends on channel operations, I think we should discuss it.

Choose a reason for hiding this comment

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

Oh I meant "channel operations" rather than "communication operations"

With the introduction of CSP for Paddle, we mimic this behavior by
creating a ***select_op***. The ***select_op*** works in conjunction
with the **channel_send_op**, **channel_receive_op**, and
**channel_close_op**.

Choose a reason for hiding this comment

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

Umm is it? I thought select_op does not use the channel ops? Doesn't it just use Channels directly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's true, I'll just say it works with ChannelHolder directly

will prefer to use the much simplier Python API.

- **fluid.Select()**: Creates a Select block and adds it to the main
program block. Within the select block, users can add cases by calling

Choose a reason for hiding this comment

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

Not main program block right? It will be in whichever block select_op sits in

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'll change this to the current block within the main program

assign(input=x, output=x_tmp)
assign(input=y, output=x)
assign(elementwise_add(x=x_tmp, y=y), output=y)
with select.case(fluid.channel_recv, quit_channel, result2):

Choose a reason for hiding this comment

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

You haven't defined result2

Choose a reason for hiding this comment

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

And quit_channel

while_cond = fill_constant(shape=[1], dtype=core.VarDesc.VarType.BOOL, value=True)
while_op = While(cond=while_cond)

with while_op.block():

Choose a reason for hiding this comment

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

While is a little tricky case - not sure I would use it for an example introducing it for the first time. BUT if you want to use to show how it can work with while, you might have to add a little more text perhaps below the example to explain why we do that funky append_op business to get out of the while. Because there is nothing normal about it.


The python select API will add the **select_op** to the current block. In addition, it will
iterate through all it's case statements and add any input variables required by case statements
into **X**. It will also create a temp variable called **case_to_execute**. This variable is

Choose a reason for hiding this comment

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

And make a note about the outputs also here, since we added that

Choose a reason for hiding this comment

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

case_to_execute is not really a tmp variable. We are explicitly passing it in. So it will modify the variable case_to_execute

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are doing it for the user when they call fluid.select() right?

Choose a reason for hiding this comment

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

Yes, so may be I just don't know what tmp variables means, you know better


Cases are represented by a **conditional_block_op** whose's condition is set as the output of
equal(**case_to_execute**, **case_index**). Since each case index is unique in this sub-block,
only once case will be executed.

Choose a reason for hiding this comment

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

Love the explanation. Concise and very accurate!

</p>

The select algorithm is inspired by golang's select routine. Please refer to
http://www.tapirgames.com/blog/golang-concurrent-select-implementation for more information.

Choose a reason for hiding this comment

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

So I realize the hard way that Tapir Liu is not the best explainer in the world, and the reason we spent so much time understanding his blog was because he didn't do the best job of summarizing that code. Reading the code brought all the clarity.

Anyways, you don't need to do anything here, this is a just a comment/thought.

…OP_DESIGN_DOC

# Conflicts:
#	doc/design/csp.md
#	doc/design/csp/csp.md
#	doc/fluid/design/concurrent/csp.md
@varunarora varunarora merged commit 873cb9b into PaddlePaddle:develop Mar 20, 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.

None yet

2 participants