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

delete_ops call should also delete variables of the op, but do not delete vars that all other ops use. #9794

Closed
gongweibao opened this issue Apr 9, 2018 · 8 comments
Assignees

Comments

@gongweibao
Copy link
Contributor

No description provided.

@gongweibao
Copy link
Contributor Author

@luotao1
Copy link
Contributor

luotao1 commented Apr 9, 2018

Dose #9384 solve this issue?

@gongweibao
Copy link
Contributor Author

A potential bugs of current implementation:

  1. A user creates a global variable LearningRate without operator to use it.
  2. Assign this variable to Adam optimizer.
    Adam optimizer can accept float or variable argument.
  3. When we use delete_ops to delete optimizers, it deletes all variable not used by other operators and so LearningRate variable is deleted.
  4. When the user uses this variable, information prompted is incomprehensible

I met this last Saturday, how do you think to avoid this?
https://github.com/gongweibao/tests/issues/2

@luotao1
Copy link
Contributor

luotao1 commented Apr 10, 2018

Since LearningRate is assigned to Adam_op in stage2, why it be deleted in stage3? Does Adam_op not be added to the program_desc?

@gongweibao
Copy link
Contributor Author

gongweibao commented Apr 10, 2018

@luotao1
Copy link
Contributor

luotao1 commented Apr 10, 2018

Does LearningRate and Adam_op in the same block? When we use delete_ops to delete operators, it deletes all variable not used by other operators in the same block.

@luotao1
Copy link
Contributor

luotao1 commented Apr 10, 2018

I have another demand that when we delete ops, we should not delete any variables.

Inspired by #9765, we should add remove_op in Python end like insert_op. The remove_op in Python will call self.desc.remove_op in C++ end. However, as our remove_op in C++ end delete variables that all other ops not use, we should implement the same thing in Python end, which make the code redundancy.

Thus, how about remove_op only remove ops, and remove_var only remove variable? If we have the demand that removing unused variables, we can use a new function do this? @typhoonzero @gongweibao

@gongweibao
Copy link
Contributor Author

It's not a perfect idea because it demands users to very careful with variables.

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

No branches or pull requests

4 participants