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

Handle optional inputs and remove more empty shape nodes in TensorRT EP #3455

Merged
merged 3 commits into from
Apr 10, 2020

Conversation

stevenlix
Copy link
Contributor

  • Remove optional inputs at (sub)graph level
    When TRT EP partitions a graph for TRT, optional input of operators become subgraph's input, which will cause problem since it doesn't have name/type/values. This PR removed optional inputs from subgraph input list.
  • Exclude empty shape related nodes from TRT subgraph
    TRT doesn't support empty shape in current release. When a model has Nonzero op, there is a chance that Nonzero's output has empty shape during runtime. This mostly happened in RCNN models. As a workaround this PR excluded empty shape related nodes from TRT subgraph. New TRT release 7.1 will fix the issue and support empty shape.

@stevenlix stevenlix requested review from jywu-msft and a team April 8, 2020 19:36
const auto name = input.second->Name();
if (!name.empty()){
meta_def->inputs.push_back(name);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: tabs

meta_def->inputs.push_back(input.second->Name());
const auto name = input.second->Name();
if (!name.empty()){
meta_def->inputs.push_back(name);
Copy link
Contributor

Choose a reason for hiding this comment

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

checking input.second->Exists() may be clearer/simpler.

@skottmckay
Copy link
Contributor

    const auto& it = fused_inputs.find(output);

There are also optional outputs. Do you need to explicitly handle those too?


Refers to: onnxruntime/core/providers/tensorrt/tensorrt_execution_provider.cc:294 in 64a3814. [](commit_id = 64a3814, deletion_comment = False)

for (const auto& tensor : init_tensors) {
graph_build.AddInitializedTensor(*(tensor.second));
}

ORT_ENFORCE(graph_build.Resolve().IsOK());
Copy link
Contributor

Choose a reason for hiding this comment

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

nit:tabs

Copy link
Member

@jywu-msft jywu-msft left a comment

Choose a reason for hiding this comment

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

merge master to get the Windows GPU CI passing.

@stevenlix stevenlix merged commit 56e8548 into master Apr 10, 2020
@stevenlix stevenlix deleted the stevenlix/onestg branch April 10, 2020 18:13
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.

3 participants