Skip to content

[quant][graphmode] Remove dedup pass#39825

Closed
jerryzh168 wants to merge 8 commits intogh/jerryzh168/339/basefrom
gh/jerryzh168/339/head
Closed

[quant][graphmode] Remove dedup pass#39825
jerryzh168 wants to merge 8 commits intogh/jerryzh168/339/basefrom
gh/jerryzh168/339/head

Conversation

@jerryzh168
Copy link
Copy Markdown
Contributor

@jerryzh168 jerryzh168 commented Jun 10, 2020

Stack from ghstack:

Summary:
Removing the pass for now since it is causing error for some models

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: D21987878

Summary:
Removing the pass for now since it is causing error for some models

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

[ghstack-poisoned]
Summary:
Removing the pass for now since it is causing error for some models

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

[ghstack-poisoned]
@dr-ci
Copy link
Copy Markdown

dr-ci Bot commented Jun 10, 2020

💊 CI failures summary and remediations

As of commit 980ad71 (more details on the Dr. CI page):


  • 1/1 failures possibly* introduced in this PR
    • 1/1 non-CircleCI failure(s)

ci.pytorch.org: 1 failed


This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions on the GitHub issue tracker or post in the (internal) Dr. CI Users group.

See how this bot performed.

This comment has been revised 21 times.

Summary:
Removing the pass for now since it is causing error for some models

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: [D21987878](https://our.internmc.facebook.com/intern/diff/D21987878)

[ghstack-poisoned]
if not all(isinstance(x, str) for x in qconfig_dict.keys()):
raise ValueError('qconfig_dict should only contain names(str) as keys.')
scripted_qconfig_dict = script_qconfig_dict(qconfig_dict)
torch._C._jit_pass_dedup_module_uses(model._c)
Copy link
Copy Markdown
Contributor

@raghuramank100 raghuramank100 Jun 11, 2020

Choose a reason for hiding this comment

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

Dont you need to remove the corresponding tests?
I thought this pass was needed to handle cases like:

  • x = self.conv1(x)
  • x = self.relu(x)
  • x = self.conv2(x)
  • x = self.relu(x)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this is a legacy pass we added for this case, but this case can be addressed with current code already. I feel this pass might not be needed if we have some (unlikely) case like:

x = self.submodule_with_states(x)
...
x = self.submodule_with_states(x)

that is: we use some submodule with states multiple times.

I only removed this from API, this pass is still there. If we decide to support the above use case at some point, we'll debug the problem and re-enable the pass.

Summary:
Removing the pass for now since it is causing error for some models

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: [D21987878](https://our.internmc.facebook.com/intern/diff/D21987878)

[ghstack-poisoned]
Summary:
Removing the pass for now since it is causing error for some models

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: [D21987878](https://our.internmc.facebook.com/intern/diff/D21987878)

[ghstack-poisoned]
Summary:
Removing the pass for now since it is causing error for some models

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: [D21987878](https://our.internmc.facebook.com/intern/diff/D21987878)

[ghstack-poisoned]
Summary:
Removing the pass for now since it is causing error for some models

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: [D21987878](https://our.internmc.facebook.com/intern/diff/D21987878)

[ghstack-poisoned]
Summary:
Removing the pass for now since it is causing error for some models

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: [D21987878](https://our.internmc.facebook.com/intern/diff/D21987878)

[ghstack-poisoned]
@facebook-github-bot
Copy link
Copy Markdown
Contributor

This pull request has been merged in 14e841c.

@facebook-github-bot facebook-github-bot deleted the gh/jerryzh168/339/head branch June 15, 2020 14:15
xwang233 pushed a commit to xwang233/pytorch that referenced this pull request Jun 20, 2020
Summary:
Pull Request resolved: pytorch#39825

Removing the pass for now since it is causing error for some models

Test Plan: Imported from OSS

Differential Revision: D21987878

fbshipit-source-id: 129aefb34754d5390a4c9d3108fa1b6c2eae5a74
laurentdupin pushed a commit to laurentdupin/pytorch that referenced this pull request Apr 24, 2026
Summary:
Pull Request resolved: pytorch#39825

Removing the pass for now since it is causing error for some models

Test Plan: Imported from OSS

Differential Revision: D21987878

fbshipit-source-id: 129aefb34754d5390a4c9d3108fa1b6c2eae5a74
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants