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
[quant][graphmode][fx] Add support for dynamic quant for RNN and RNNCell #49126
Conversation
Summary: Test Plan: python test/test_quantization.py TestQuantizeFxOps.test_rnn python test/test_quantization.py TestQuantizeFxOps.test_rnn_cell Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]
…NN and RNNCell" Summary: Test Plan: python test/test_quantization.py TestQuantizeFxOps.test_rnn python test/test_quantization.py TestQuantizeFxOps.test_rnn_cell Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]
Summary: Test Plan: python test/test_quantization.py TestQuantizeFxOps.test_rnn python test/test_quantization.py TestQuantizeFxOps.test_rnn_cell Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 810a87cfb19308fa036220936e456df46c5714f5 Pull Request resolved: #49126
💊 CI failures summary and remediationsAs of commit ff4353e (more details on the Dr. CI page):
🕵️ 3 new failures recognized by patternsThe following CI failures do not appear to be due to upstream breakages: pytorch_linux_xenial_cuda10_2_cudnn7_py3_gcc7_test1 (1/3)Step: "Run tests" (full log | diagnosis details | 🔁 rerun)
|
} | ||
model_graph = prepare_fx(model_graph, graph_qconfig_dict) | ||
model_graph = convert_fx(model_graph) | ||
self.assertEqual(model_eager(sample_input), model_graph(sample_input)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to test for serialization here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be the same as eager mode module, I'm not very familiar, are we using state_dict?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or are you referring to checkScriptable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added checkScriptable here, but in general we'll do e2e test in TestQuantizeFxModels
@@ -124,6 +124,19 @@ def get_static_quant_module_class(float_module_class, additional_static_quant_ma | |||
" does not have a corresponding quantized module class" | |||
return static_quant_module_class | |||
|
|||
def get_dynamic_quant_module_class(float_module_class, additional_dynamic_quant_mapping=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would be great to add types to function I/O
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can add it in a separate PR I think, all other functions in this file are not typed yet
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's not blocking this PR, but would be awesome if we started adding these as we go, at least to function I/O. We don't have to wait for a file to have existing type annots to add more. This also distributes the cost of adding them to everyone, as opposed to one person.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, fully agree that we should add types as we change code. I'm saying I plan to add it in a separate PR, or are you suggesting to add the type annotations for the functions in this file in this PR?
…NN and RNNCell" Summary: Test Plan: python test/test_quantization.py TestQuantizeFxOps.test_rnn python test/test_quantization.py TestQuantizeFxOps.test_rnn_cell Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D25449047](https://our.internmc.facebook.com/intern/diff/D25449047) [ghstack-poisoned]
This pull request has been merged in 882eb0f. |
Stack from ghstack:
Summary:
Test Plan:
python test/test_quantization.py TestQuantizeFxOps.test_rnn
python test/test_quantization.py TestQuantizeFxOps.test_rnn_cell
Reviewers:
Subscribers:
Tasks:
Tags:
Differential Revision: D25449047