Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Support for Tensorflow eager mode #670
Support for Tensorflow eager mode #670
Changes from 14 commits
c7a6843
40cbd5a
5763876
642736b
2c80222
f4e134b
4f522b8
da38b1b
2179d1e
a6c60fd
8f9ff60
73e3e72
e101014
5a5adf2
ffa00c5
c7c1498
5140e92
e4b62b8
8aeb8f4
b3467f4
d9e113a
d672eff
c0f8a27
4306909
365629a
977b9d1
1c1eadc
b1de533
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you rename this to
broadcast_variables
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure. i renamed to
broadcast_variable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you refactor this per the changes to
DistributedOptimizer
in #704? Basically, without usingtf.contrib.eager.defun
, this will be very slow.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok. i updated it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should pass
device_dense
and etc. correctly.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i updated it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it necessary to make eager execution a special case here? For some things, like the upcoming autotuning framework, we rely on differentiating tensors by their name for things like detecting loops.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when eager mode, tensor.name() is meaningless.So i add this code.reference here
If we need to distinguish tensor, we need to use something that changes to tensor.name ().
I am looking into that way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. One issue here is that older versions of TF <1.7 will not support
tf.executing_eagerly()
. I made a change in #704 to address this by adding a new utility function. I think this should work for now.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. I update it