-
Notifications
You must be signed in to change notification settings - Fork 63
Shrestha/var in compute #388
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
Conversation
…ensorflow/ngraph-bridge into shrestha/prefetch_right_inputs
|
|
||
| // For graphs that were run through AOT | ||
| // Graph rewrite is not done, and there is no entry in catalog | ||
| // If there is not entry in catalog all outputs need to be copied |
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.
typo not -- > no
sayantan-nervana
left a comment
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.
One minor comment about reusing ng::join
| m_use_parallel_executor = backend->executable_can_create_tensors(); | ||
| #else | ||
| m_use_parallel_executor = false; | ||
| #endif |
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.
note to self: important section here.
| void* dst_ptr = (void*)DMAHelper::base(tf_output_tensors[output_index]); | ||
| ng_outputs[output_index]->read( | ||
| dst_ptr, ng_outputs[output_index]->get_element_count() * | ||
| ng_outputs[output_index]->get_element_type().size()); |
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.
note to self: instead of calling read on everything, call read only on output_indexes_to_be_copied
| ss << val << " "; | ||
| } | ||
| cout << ss.str() << endl; | ||
| } |
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.
there is ng::join. Maybe we don't need this.
cout << title << " " << ng::join(input_vector) << "\n";
vishakha-nervana
left a comment
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.
LGTM, Do we have any c++ test for this use case?
…ow/ngraph-bridge into shrestha/var_in_compute
sayantan-nervana
left a comment
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.
LGTM
Uh oh!
There was an error while loading. Please reload this page.