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 go_op design doc #9389

Merged
merged 3 commits into from
Mar 27, 2018
Merged

Conversation

cs2be
Copy link
Contributor

@cs2be cs2be commented Mar 26, 2018

No description provided.

Copy link

@varunarora varunarora left a comment

Choose a reason for hiding this comment

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

Thoughts...


## Current Limitations

####<a name="block-captures"></a>Scopes and block captures:

Choose a reason for hiding this comment

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

Space after ####


## How it Works

Similair to other control blocks, go_op will create a sub block and add it

Choose a reason for hiding this comment

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

"Similar"

## Introduction

The **go_op** allows user's of PaddlePaddle to run program blocks on a detached
thread. It works in conjuction with CSP operators (channel_send,

Choose a reason for hiding this comment

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

just a quick formatting note: if you wrap terms like channel_send with the character next to 1 on your keyboard, it becomes much easier to read channel_send throughout the doc

the executor.run method (along with a newly created local scope) on a detached
thread.

```

Choose a reason for hiding this comment

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

Provide some commentary into this programdesc dump if you want people to use it as a reference

scope, it may receive a segmentation fault because the parent scope may have
been deleted.

We need to implement block closures in order to prevent access to parent

Choose a reason for hiding this comment

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

You can also say that we explicitly enforce this.


Please refer to [Closure issue](https://github.com/PaddlePaddle/Paddle/issues/8502)
for more details.

Choose a reason for hiding this comment

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

If you want to make a note on how to debug the thread, this would also be a good point of info. The way you told me to do the try-catches and eventually remove the running of things in the thread entirely

Choose a reason for hiding this comment

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

Another section or para I think would be useful is the choice of threading system. And that for now we are not using green threads.

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 mention about green threads. I won't mention the try catch because the user will only run into this issue if they run gtest

#### Backward Propegation:

go_op currently does not support backwards propagation. Please use go_op with
non training operators.

Choose a reason for hiding this comment

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

Where are you going to write a detailed explanation of the considerations and challenges associated with backprop in go? This wouldn't be a terrible place.

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 leave this for later...

@cs2be cs2be merged commit ab5a356 into PaddlePaddle:develop Mar 27, 2018
@cs2be cs2be deleted the GO_OP_DESIGN branch March 27, 2018 00:17
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