Missing outputs of input placeholder #1045
Conversation
nlpng
changed the title
|
This PR needs Approvals as follows.
Please choose reviewers and requet reviews! Click to see how to approve each reviewsYou can approve this PR by triggered comments as follows.
See all trigger commentsPlease replace [Target] to review target
|
blueoil/converter/core/optimizer.py
Outdated
| reserved_placeholder_ops = [] | ||
| for out_op in placeholder[0].output_op_list: | ||
| if out_op not in to_be_removed: | ||
| reserved_placeholder_ops.append(out_op) |
There was a problem hiding this comment.
Usign list comprehension is better for readability and efficiency.
reserved_placeholder_ops = [
out_op
for out_op in placeholder[0].output_op_list
if out_op not in to_be_removed
]or
reserved_placeholder_ops = [
out_op for out_op in placeholder[0].output_op_list
if out_op not in to_be_removed
]
blueoil/converter/core/optimizer.py
Outdated
| reserved_placeholder_ops = [ | ||
| out_op for out_op in placeholder[0].output_op_list | ||
| if out_op not in to_be_removed | ||
| ] | ||
| placeholder[0].remove_output('output') | ||
| placeholder[0].add_output('output', pe) | ||
| for add_to_placeholder in reserved_placeholder_ops: | ||
| placeholder[0].add_output('output', add_to_placeholder) |
There was a problem hiding this comment.
Oh sorry, my previous review was bad...
Those two loops are not needed to be separated. Could you make them into one?
There was a problem hiding this comment.
Thanks, I fixed in commit db82531, not sure it is the fix you are looking for, but now it is only one for loop.
|
/ready |
|
⏳Merge job is queued... |
What this patch does to fix the issue.
This PR is to deal with cases that the input placeholder has two or more shared branches, as the branches maybe removed during the optimization pass.
Link to any relevant issues or pull requests.