[ONNX] De-duplicate initializers #68202
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
CI Flow Status⚛️ CI FlowRuleset - Version:
You can add a comment to the PR and tag @pytorchbot with the following commands: # ciflow rerun, "ciflow/default" will always be added automatically
@pytorchbot ciflow rerun
# ciflow rerun with additional labels "-l <ciflow/label_name>", which is equivalent to adding these labels manually and trigger the rerun
@pytorchbot ciflow rerun -l ciflow/scheduled -l ciflow/slowFor more information, please take a look at the CI Flow Wiki. |
🔗 Helpful links
💊 CI failures summary and remediationsAs of commit 0793a75 (more details on the Dr. CI page):
🕵️ 2 new failures recognized by patternsThe following CI failures do not appear to be due to upstream breakages:
|
| Job | Step | Action |
|---|---|---|
 pytorch_linux_xenial_py3_6_gcc5_4_test |
Report results | 🔁 rerun |
This comment was automatically generated by Dr. CI (expand for details).
Please report bugs/suggestions to the (internal) Dr. CI Users group.
facebook-github-bot
added
the
oncall: jit
garymm
requested changes
Nov 12, 2021
| auto valsToParamsMap = buildValueToParamsMap(b, paramsDict); | ||
| fuseConvBatchNorm(b, valsToParamsMap); | ||
| if (opset_version >= OPSET_VERSION_15) { | ||
| // Apply de-duplication after opset 15 for backward compatibility. |
There was a problem hiding this comment.
Please elaborate on this comment. I don't see anything in the ONNX 1.10 release notes that seems to relate to duplicate initializers.
There was a problem hiding this comment.
This is open to discussion. Not related to ONNX release, but we are changing default behavior of exporter regarding initializers, so I'm wondering if we should only apply it on newer opsets for BC purpose. This applies for other future passes of similar types as well. Would love to hear your thoughts on this.
There was a problem hiding this comment.
I think unless we have reason to believe this will not work on older runtimes, we should not guard the behavior.
I suggest we test this against Caffe2 using test_pytorch_onnx_caffe2.py. If we want to be extra careful you could also install an older version of ORT and run test_pytorch_onnx_onnxruntime.py.
If it passes both of those, then I think it's likely that we don't need to guard this behavior.
WDYT?
There was a problem hiding this comment.
Ok we can give a try. For general usage I'm not too worried. If there are issues they are considered bugs and we can resolve them. I'm slightly worried against user custom behaviors on top of initializer/parameters.
There was a problem hiding this comment.
The test_operator.py result is one example, the change in parameters may appear confusing for users. For better readability I updated the logic to insert identity node instead of directly replacing initializers.
BowenBao
force-pushed
the
deduplicate_initialiers
branch
from
841a2da to
402b276
Compare
garymm
approved these changes
Nov 18, 2021
|
Still LGTM with identity nodes. Feel free to merge once tests pass. |
BowenBao
force-pushed
the
deduplicate_initialiers
branch
from
47682e7 to
bbbb02f
Compare
BowenBao
added a commit
that referenced
this pull request
Dec 7, 2021
ScriptModule export introduces duplicated ONNX initializers for shared weights, unnecessarily increases ONNX model size. This PR de-duplicates ONNX initializers for model exported in eval mode, by checking if the underlying tensors share the same `data_ptr`, `strides` and `sizes`. Co-authored-by: BowenBao <bowbao@microsoft.com> [ghstack-poisoned]
BowenBao
added a commit
to BowenBao/pytorch
that referenced
this pull request
Jan 5, 2022
ScriptModule export introduces duplicated ONNX initializers for shared weights, unnecessarily increases ONNX model size. This PR de-duplicates ONNX initializers for model exported in eval mode, by checking if the underlying tensors share the same `data_ptr`, `strides` and `sizes`. Co-authored-by: BowenBao <bowbaomicrosoft.com> ghstack-source-id: d17dfa4 Pull Request resolved: pytorch#69547
BowenBao
added a commit
that referenced
this pull request
Jan 7, 2022
ScriptModule export introduces duplicated ONNX initializers for shared weights, unnecessarily increases ONNX model size. This PR de-duplicates ONNX initializers for model exported in eval mode, by checking if the underlying tensors share the same `data_ptr`, `strides` and `sizes`. Co-authored-by: BowenBao <bowbaomicrosoft.com> Differential Revision: [D32994271](https://our.internmc.facebook.com/intern/diff/D32994271) [ghstack-poisoned]
BowenBao
added a commit
that referenced
this pull request
Jan 7, 2022
ScriptModule export introduces duplicated ONNX initializers for shared weights, unnecessarily increases ONNX model size. This PR de-duplicates ONNX initializers for model exported in eval mode, by checking if the underlying tensors share the same `data_ptr`, `strides` and `sizes`. Co-authored-by: BowenBao <bowbaomicrosoft.com> Differential Revision: [D32994271](https://our.internmc.facebook.com/intern/diff/D32994271) [ghstack-poisoned]
BowenBao
added a commit
that referenced
this pull request
Jan 11, 2022
ScriptModule export introduces duplicated ONNX initializers for shared weights, unnecessarily increases ONNX model size. This PR de-duplicates ONNX initializers for model exported in eval mode, by checking if the underlying tensors share the same `data_ptr`, `strides` and `sizes`. Co-authored-by: BowenBao <bowbaomicrosoft.com> Differential Revision: [D32994271](https://our.internmc.facebook.com/intern/diff/D32994271) [ghstack-poisoned]
BowenBao
added a commit
that referenced
this pull request
Jan 11, 2022
ScriptModule export introduces duplicated ONNX initializers for shared weights, unnecessarily increases ONNX model size. This PR de-duplicates ONNX initializers for model exported in eval mode, by checking if the underlying tensors share the same `data_ptr`, `strides` and `sizes`. Co-authored-by: BowenBao <bowbaomicrosoft.com> Differential Revision: [D32994271](https://our.internmc.facebook.com/intern/diff/D32994271) [ghstack-poisoned]
BowenBao
added a commit
to BowenBao/pytorch
that referenced
this pull request
Jan 21, 2022
ScriptModule export introduces duplicated ONNX initializers for shared weights, unnecessarily increases ONNX model size. This PR de-duplicates ONNX initializers for model exported in eval mode, by checking if the underlying tensors share the same `data_ptr`, `strides` and `sizes`. Co-authored-by: BowenBao <bowbaomicrosoft.com> ghstack-source-id: d17dfa4 Pull Request resolved: pytorch#69547
BowenBao
added a commit
to BowenBao/pytorch
that referenced
this pull request
Jan 21, 2022
ScriptModule export introduces duplicated ONNX initializers for shared weights, unnecessarily increases ONNX model size. This PR de-duplicates ONNX initializers for model exported in eval mode, by checking if the underlying tensors share the same `data_ptr`, `strides` and `sizes`. Co-authored-by: BowenBao <bowbaomicrosoft.com> ghstack-source-id: d17dfa4 Pull Request resolved: pytorch#69547
BowenBao
added a commit
to BowenBao/pytorch
that referenced
this pull request
Jan 31, 2022
ScriptModule export introduces duplicated ONNX initializers for shared weights, unnecessarily increases ONNX model size. This PR de-duplicates ONNX initializers for model exported in eval mode, by checking if the underlying tensors share the same `data_ptr`, `strides` and `sizes`. Co-authored-by: BowenBao <bowbaomicrosoft.com> ghstack-source-id: d17dfa4 Pull Request resolved: pytorch#69547
garymm
pushed a commit
to garymm/pytorch
that referenced
this pull request
Feb 9, 2022
ScriptModule export introduces duplicated ONNX initializers for shared weights, unnecessarily increases ONNX model size. This PR de-duplicates ONNX initializers for model exported in eval mode, by checking if the underlying tensors share the same `data_ptr`, `strides` and `sizes`. Co-authored-by: BowenBao <bowbaomicrosoft.com> ghstack-source-id: d17dfa4 Pull Request resolved: pytorch#69547
BowenBao
added a commit
that referenced
this pull request
Feb 10, 2022
ScriptModule export introduces duplicated ONNX initializers for shared weights, unnecessarily increases ONNX model size. This PR de-duplicates ONNX initializers for model exported in eval mode, by checking if the underlying tensors share the same `data_ptr`, `strides` and `sizes`. Co-authored-by: BowenBao <bowbaomicrosoft.com> Differential Revision: [D32994271](https://our.internmc.facebook.com/intern/diff/D32994271) [ghstack-poisoned]
BowenBao
added a commit
that referenced
this pull request
Feb 10, 2022
ScriptModule export introduces duplicated ONNX initializers for shared weights, unnecessarily increases ONNX model size. This PR de-duplicates ONNX initializers for model exported in eval mode, by checking if the underlying tensors share the same `data_ptr`, `strides` and `sizes`. Co-authored-by: BowenBao <bowbaomicrosoft.com> Differential Revision: [D32994271](https://our.internmc.facebook.com/intern/diff/D32994271) [ghstack-poisoned]
facebook-github-bot
pushed a commit
that referenced
this pull request
Feb 11, 2022
Summary: Pull Request resolved: #69547 ScriptModule export introduces duplicated ONNX initializers for shared weights, unnecessarily increases ONNX model size. This PR de-duplicates ONNX initializers for model exported in eval mode, by checking if the underlying tensors share the same `data_ptr`, `strides` and `sizes`. Test Plan: Imported from OSS Reviewed By: msaroufim Differential Revision: D32994271 Pulled By: malfet fbshipit-source-id: 10ac66638b6255890875272472aa9ed07a5b1d9a Co-authored-by: BowenBao <bowbao@microsoft.com>
pytorchmergebot
pushed a commit
that referenced
this pull request
Feb 11, 2022
Summary: Pull Request resolved: #69547 ScriptModule export introduces duplicated ONNX initializers for shared weights, unnecessarily increases ONNX model size. This PR de-duplicates ONNX initializers for model exported in eval mode, by checking if the underlying tensors share the same `data_ptr`, `strides` and `sizes`. Test Plan: Imported from OSS Reviewed By: msaroufim Differential Revision: D32994271 Pulled By: malfet fbshipit-source-id: 10ac66638b6255890875272472aa9ed07a5b1d9a Co-authored-by: BowenBao <bowbao@microsoft.com> (cherry picked from commit d7cbde9)
fxmarty
reviewed
Aug 31, 2023
Comment on lines
+20
to
+22
| if ((valsToParamsMap.find(v1) == valsToParamsMap.end()) || | ||
| (valsToParamsMap.find(v2) == valsToParamsMap.end())) { | ||
| return false; |
There was a problem hiding this comment.
Yes related to #108342. I think I misread the code, not super familiar with C++.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
ScriptModule export introduces duplicated ONNX initializers for shared weights, unnecessarily increases ONNX model size. This PR de-duplicates ONNX initializers for model exported in eval mode, by checking if the underlying tensors share the same
data_ptr,stridesandsizes.