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

remove vars when remove ops #9384

Merged
merged 2 commits into from
Mar 28, 2018
Merged

remove vars when remove ops #9384

merged 2 commits into from
Mar 28, 2018

Conversation

luotao1
Copy link
Contributor

@luotao1 luotao1 commented Mar 26, 2018

when removing an op, the vars of this op should be removed at the same time:

  • output vars of this op: remove all.
  • input vars of this op: remove the input vars which are not used by other ops.

add a simple unit test to check RemoveOp method.

@luotao1 luotao1 added the 预测 原名Inference,包含Capi预测问题等 label Mar 26, 2018
@typhoonzero
Copy link
Contributor

Is there a case that the output of the removed op is used by other op?

@luotao1
Copy link
Contributor Author

luotao1 commented Mar 27, 2018

If the output of removed op is the input of other ops, then other ops should adjust their inputs. If the output of removed op is the output of other ops, although this case has not been seen now, but these output vars should be remained, how do you think about it ?

@luotao1 luotao1 requested a review from qingqing01 March 27, 2018 01:59
@qingqing01
Copy link
Contributor

qingqing01 commented Mar 27, 2018

Is there a case that the output of the removed op is used by other op?

I think this case is in existence. Maybe two ways to remove var:

  1. remove vars specified by name ?
  2. go through ProgramDesc to check whether some unused vars need to be removed ?

@typhoonzero
Copy link
Contributor

typhoonzero commented Mar 27, 2018

If we want to make this a general function, it's quite complex, see #9080 and https://en.wikipedia.org/wiki/Static_single_assignment_form. We must build a SSA graph before head so that two ops have same outputs ( write to var ) won't affect each other.

For the simple implementation, I think just check if the output is used by other ops and do not remove them should work.

@qingqing01

remove vars specified by name ?

For generate inference programdesc, if remove_op can also remove unused vars, it's simpler to transpile a program for inference, I think.

@luotao1
Copy link
Contributor Author

luotao1 commented Mar 27, 2018

I agree with the simple implementation for generating inference program-desc at first.
And I adjust the remove rule of variables: for either input or output variable, if it is also an input or output variable of other ops, we should remain it.

Copy link
Contributor

@typhoonzero typhoonzero left a comment

Choose a reason for hiding this comment

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

LGTM++

@luotao1 luotao1 merged commit 857a899 into PaddlePaddle:develop Mar 28, 2018
@luotao1 luotao1 deleted the removeVar branch March 28, 2018 12:54
@Xreki Xreki added this to Basic Usage (DONE) in Inference Framework Apr 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
预测 原名Inference,包含Capi预测问题等
Projects
No open projects
Inference Framework
Basic Usage (DONE)
Development

Successfully merging this pull request may close these issues.

None yet

3 participants