Skip to content

Conversation

@sayantan-nervana
Copy link
Contributor

@sayantan-nervana sayantan-nervana commented Dec 6, 2019

For certain backends (even if their executable can create tensors), use backends to create tensors

@sayantan-nervana sayantan-nervana added 🌋 experimental This is an experimental PR. May not be merged. wip Work in progress labels Dec 6, 2019
@sayantan-nervana sayantan-nervana removed the wip Work in progress label Dec 10, 2019
Copy link
Contributor

@kanvi-nervana kanvi-nervana left a comment

Choose a reason for hiding this comment

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

A minor comment other looks good

std::shared_ptr<ngraph::runtime::Executable> exec,
std::vector<tensorflow::PersistentTensor>& tf_output_tensors) {
auto itr = m_out_persistents.find(exec);
if (itr == m_out_persistents.end()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could use PersistentOutputsExist

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Then I'd be doing 2 searches. One in PersistentOutputsExist and one in again to actually return the item. Right now I do only one search to find the appropriate iterator

std::shared_ptr<ngraph::runtime::Executable> exec,
std::vector<tensorflow::PersistentTensor> persistent_tensors) {
auto itr = m_out_persistents.find(exec);
if (itr != m_out_persistents.end()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could use PersistentOutputsExist

Copy link
Contributor

@shresthamalik shresthamalik left a comment

Choose a reason for hiding this comment

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

LGTM

@shresthamalik
Copy link
Contributor

Please look into the failing pipelines

Copy link
Contributor

@vishakha-nervana vishakha-nervana left a comment

Choose a reason for hiding this comment

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

LGTM

@sayantan-nervana
Copy link
Contributor Author

Please look into the failing pipelines
The usual suspects pipelines are failing with the "gcc7" and "whl not found errors"

@sayantan-nervana sayantan-nervana added ready to merge This PR is the next in the queue. fully reviewed and removed 🌋 experimental This is an experimental PR. May not be merged. labels Dec 12, 2019
@sayantan-nervana sayantan-nervana merged commit 16f8811 into r0.19 Dec 12, 2019
@ashahba ashahba deleted the sarkars/backend_create_tensor branch April 24, 2020 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants