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

[Bug Fix]clear preTopology's output while cloneCells #2585

Merged
merged 2 commits into from
Jul 10, 2018

Conversation

qiuxin2012
Copy link
Contributor

@qiuxin2012 qiuxin2012 commented Jul 6, 2018

What changes were proposed in this pull request?

Recurrent cost too many memories, find it's caused by cloneCells.
clear preTopology's output while cloneCells.

With this fix, 9-depth deepspeech2 model forward a input tensor of size (1, 1, 13, 3000) cost only 1GB memory.
Without this fix, the model will cost about 80GB memory.

How was this patch tested?

unit tests, manual tests

@qiuxin2012 qiuxin2012 requested a review from yiheng July 6, 2018 08:34
@qiuxin2012 qiuxin2012 changed the title clear preTopology's output while cloneCells [Bug Fix]clear preTopology's output while cloneCells Jul 6, 2018
@yiheng
Copy link
Contributor

yiheng commented Jul 9, 2018

LGTM. Jenkins result?

@qiuxin2012
Copy link
Contributor Author

@yiheng failed, some times preTopology is null.

@qiuxin2012
Copy link
Contributor Author

jenkins passed

@yiheng yiheng merged commit 11d4942 into intel-analytics:master Jul 10, 2018
@zhichao-li
Copy link
Contributor

zhichao-li commented Jul 16, 2018

@qiuxin2012 @yiheng
Would it be better if we set preTopology of a cell to be null in add method as there's weights, grad or other stuff within a preTopology other than output

  override def add(module: AbstractModule[_ <: Activity, _ <: Activity, T]): this.type = {
    if (topology.preTopology != null) {
      val tmp = topology.preTopology.cloneModule()
      topology.preTopology = null 
      preTopology = TimeDistributed(tmp, maskZero = maskZero)
    }

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