Skip to content

Enhance exporting torch.minimum() function to ONNX so it can handle parameters with different dtype.#75861

Closed
fatcat-z wants to merge 2 commits intopytorch:masterfrom
fatcat-z:support_minimum_op
Closed

Enhance exporting torch.minimum() function to ONNX so it can handle parameters with different dtype.#75861
fatcat-z wants to merge 2 commits intopytorch:masterfrom
fatcat-z:support_minimum_op

Conversation

@fatcat-z
Copy link
Collaborator

@fatcat-z fatcat-z commented Apr 15, 2022 •
edited by garymm
Loading

Handle the case that the parameters of torch.minimum() have different dtypes. Add tests as well.

Fixes #76022

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Apr 15, 2022 •
edited
Loading

🔗 Helpful links

💊 CI failures summary and remediations

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

  • 1/1 failures introduced in this PR

🕵️ 1 new failure recognized by patterns

The following CI failures do not appear to be due to upstream breakages

See GitHub Actions build pull / linux-xenial-py3.7-gcc5.4 / test (backwards_compat, 1, 1, linux.2xlarge) (1/1)

Step: "Test" (full log | diagnosis details | 🔁 rerun)

2022-04-19T11:39:52.2800513Z The PR is introduc...m to confirm whether this change is wanted or not. 
2022-04-19T11:39:52.2784717Z processing existing schema:  text(__torch__.torch.classes.profiling.SourceRef _0) -> (str _0)
2022-04-19T11:39:52.2786445Z processing existing schema:  count(__torch__.torch.classes.profiling.InstructionStats _0) -> (int _0)
2022-04-19T11:39:52.2788053Z processing existing schema:  duration_ns(__torch__.torch.classes.profiling.InstructionStats _0) -> (int _0)
2022-04-19T11:39:52.2789796Z processing existing schema:  source(__torch__.torch.classes.profiling.SourceStats _0) -> (__torch__.torch.classes.profiling.SourceRef _0)
2022-04-19T11:39:52.2792241Z processing existing schema:  line_map(__torch__.torch.classes.profiling.SourceStats _0) -> (Dict(int, __torch__.torch.classes.profiling.InstructionStats) _0)
2022-04-19T11:39:52.2793059Z processing existing schema:  __init__(__torch__.torch.classes.profiling._ScriptProfile _0) -> (NoneType _0)
2022-04-19T11:39:52.2794916Z processing existing schema:  enable(__torch__.torch.classes.profiling._ScriptProfile _0) -> (NoneType _0)
2022-04-19T11:39:52.2796318Z processing existing schema:  disable(__torch__.torch.classes.profiling._ScriptProfile _0) -> (NoneType _0)
2022-04-19T11:39:52.2798793Z processing existing schema:  _dump_stats(__torch__.torch.classes.profiling._ScriptProfile _0) -> (__torch__.torch.classes.profiling.SourceStats[] _0)
2022-04-19T11:39:52.2800098Z processing existing schema:  __init__(__torch__.torch.classes.dist_rpc.WorkerInfo _0, str _1, int _2) -> (NoneType _0)
2022-04-19T11:39:52.2800513Z The PR is introducing backward incompatible changes to the operator library. Please contact PyTorch team to confirm whether this change is wanted or not. 
2022-04-19T11:39:52.2800525Z 
2022-04-19T11:39:52.2800662Z Broken ops: [
2022-04-19T11:39:52.2801229Z 	aten::_validate_sparse_compressed_tensor_args(Tensor crow_indices, Tensor col_indices, Tensor values, int[] size, int layout) -> ()
2022-04-19T11:39:52.2801355Z ]
2022-04-19T11:39:52.3848079Z + cleanup
2022-04-19T11:39:52.3848339Z + retcode=1
2022-04-19T11:39:52.3848551Z + set +x
2022-04-19T11:39:52.3886873Z ##[error]Process completed with exit code 1.
2022-04-19T11:39:52.3931041Z ##[group]Run pytorch/pytorch/.github/actions/get-workflow-job-id@master
2022-04-19T11:39:52.3931299Z with:
This comment was automatically generated by Dr. CI (expand for details).

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

@facebook-github-bot facebook-github-bot added the oncall: jit Add this issue/PR to JIT oncall triage queue label Apr 15, 2022
@garymm
Copy link
Collaborator

garymm commented Apr 18, 2022

Please reference the GitHub issue this is fixing in the PR description.

@thiagocrepaldi
Copy link
Collaborator

thiagocrepaldi commented Apr 18, 2022 

022-04-15T07:04:37.8473620Z   self._warn("No data was collected.", slug="no-data-collected")
2022-04-15T07:04:38.5487969Z 
2022-04-15T07:04:38.5487980Z 
2022-04-15T07:04:38.5490703Z =================================== FAILURES ===================================
2022-04-15T07:04:38.5491294Z �[31m�[1m_________________ TestONNXRuntime_opset14.test_minimum_dtypes __________________�[0m
2022-04-15T07:04:38.5491560Z Traceback (most recent call last):
2022-04-15T07:04:38.5491844Z   File "/var/lib/jenkins/workspace/test/onnx/test_pytorch_common.py", line 54, in wrapper
2022-04-15T07:04:38.5492210Z     return func(self)
2022-04-15T07:04:38.5492502Z   File "/var/lib/jenkins/workspace/test/onnx/test_pytorch_onnx_onnxruntime.py", line 7885, in test_minimum_dtypes
2022-04-15T07:04:38.5495104Z     self.run_test(MinimumModel(), (x, y))
2022-04-15T07:04:38.5495665Z   File "/var/lib/jenkins/workspace/test/onnx/test_pytorch_onnx_onnxruntime.py", line 333, in run_test
2022-04-15T07:04:38.5496218Z     _run_test(script_model, scripting_remained_onnx_input_idx, flatten=False)
2022-04-15T07:04:38.5496549Z   File "/var/lib/jenkins/workspace/test/onnx/test_pytorch_onnx_onnxruntime.py", line 322, in _run_test
2022-04-15T07:04:38.5496808Z     flatten=flatten)
2022-04-15T07:04:38.5497096Z   File "/var/lib/jenkins/workspace/test/onnx/test_pytorch_onnx_onnxruntime.py", line 176, in run_model_test
2022-04-15T07:04:38.5498047Z     output_names=output_names, fixed_batch_size=fixed_batch_size, training=training)
2022-04-15T07:04:38.5498447Z   File "/var/lib/jenkins/workspace/test/onnx/test_pytorch_onnx_onnxruntime.py", line 100, in convert_to_onnx
2022-04-15T07:04:38.5498867Z     ort_sess = onnxruntime.InferenceSession(f.getvalue(), so, providers=_ORT_PROVIDERS)
2022-04-15T07:04:38.5499398Z   File "/var/lib/jenkins/.local/lib/python3.7/site-packages/onnxruntime/capi/onnxruntime_inference_collection.py", line 335, in __init__
2022-04-15T07:04:38.5499779Z     self._create_inference_session(providers, provider_options, disabled_optimizers)
2022-04-15T07:04:38.5500273Z   File "/var/lib/jenkins/.local/lib/python3.7/site-packages/onnxruntime/capi/onnxruntime_inference_collection.py", line 381, in _create_inference_session
2022-04-15T07:04:38.5500672Z     sess.initialize_session(providers, provider_options, disabled_optimizers)
2022-04-15T07:04:38.5501536Z onnxruntime.capi.onnxruntime_pybind11_state.NotImplemented: [ONNXRuntimeError] : 9 : NOT_IMPLEMENTED : Could not find an implementation for Min(13) node with name 'Min_1'

@fatcat-z CI is failing with this change, please address it

@thiagocrepaldi thiagocrepaldi added module: onnx Related to torch.onnx onnx-needs-info needs information from the author / reporter before ONNX team can take action labels Apr 18, 2022
@thiagocrepaldi thiagocrepaldi self-assigned this Apr 18, 2022
@thiagocrepaldi thiagocrepaldi removed the onnx-needs-info needs information from the author / reporter before ONNX team can take action label Apr 19, 2022
@thiagocrepaldi
Copy link
Collaborator

thiagocrepaldi commented Apr 19, 2022

@pytorchmergebot merge this

@pytorchmergebot
Copy link
Collaborator

pytorchmergebot commented Apr 19, 2022

Merge failed due to Matched rule superuser, but it was not reviewed yet by any of:husthyc,naveedgol,mthrok,zou3519,teytaud, ...
Raised by https://github.com/pytorch/pytorch/actions/runs/2190506382

@thiagocrepaldi thiagocrepaldi added topic: improvements topic category onnx-needs-import This PR is related to ONNX, but touches files outside of merge rule patterns, and hence needs import labels Apr 19, 2022
@thiagocrepaldi thiagocrepaldi mentioned this pull request Apr 19, 2022
Closed
@thiagocrepaldi
Copy link
Collaborator

thiagocrepaldi commented Apr 19, 2022

@fatcat-z could you rebase this PR with upstream master? We just got a change checked in that allow us to modify "aten/src/ATen/core/interned_strings.h" and merge it quickly

@thiagocrepaldi
Copy link
Collaborator

thiagocrepaldi commented Apr 19, 2022

@pytorchmergebot merge this

@pytorchmergebot
Copy link
Collaborator

pytorchmergebot commented Apr 19, 2022

Merge failed due to Matched rule ONNX exporter, but it was not reviewed yet by any of:BowenBao,garymm
Raised by https://github.com/pytorch/pytorch/actions/runs/2190790699

@thiagocrepaldi thiagocrepaldi requested a review from garymm April 19, 2022 16:18
garymm
garymm approved these changes Apr 19, 2022
View reviewed changes
Copy link
Collaborator

@garymm garymm left a comment

Choose a reason for hiding this comment

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

@pytorchbot merge this

@pytorch-bot
Copy link

pytorch-bot bot commented Apr 19, 2022

+1

@github-actions
Copy link
Contributor

github-actions bot commented Apr 19, 2022

Hey @fatcat-z.
You've committed this PR, but it does not have both a 'release notes: ...' and 'topics: ...' label. Please add one of each to the PR. The 'release notes: ...' label should represent the part of PyTorch that this PR changes (fx, autograd, distributed, etc) and the 'topics: ...' label should represent the kind of PR it is (not user facing, new feature, bug fix, perf improvement, etc). The list of valid labels can be found here for the 'release notes: ...' and here for the 'topics: ...'.
For changes that are 'topic: not user facing' there is no need for a release notes label.

@thiagocrepaldi thiagocrepaldi added release notes: onnx torch.onnx related changes that should show up in the release notes and removed onnx-needs-import This PR is related to ONNX, but touches files outside of merge rule patterns, and hence needs import labels Apr 19, 2022
facebook-github-bot pushed a commit that referenced this pull request Apr 19, 2022
@fatcat-z @facebook-github-bot
Enhance exporting torch.minimum() function to ONNX so it can handle p…
37cf98b 
…arameters with different dtype. (#75861)

Summary:
Handle the case that the parameters of torch.minimum() have different dtypes. Add tests as well.

Fixes #76022

Pull Request resolved: #75861
Approved by: https://github.com/thiagocrepaldi, https://github.com/garymm

Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/166568d49f7479f46d609534b505b38fb48c71eb

Reviewed By: seemethere

Differential Revision: D35751458

fbshipit-source-id: ea0eb817d25d9b96db1e389bb03c2c6955f58128
@fatcat-z fatcat-z deleted the support_minimum_op branch April 20, 2022 01:51
malfet pushed a commit that referenced this pull request Apr 20, 2022
@fatcat-z @malfet
Enhance exporting torch.minimum() function to ONNX so it can handle p…
f273961 
…arameters with different dtype.

Handle the case that the parameters of torch.minimum() have different dtypes. Add tests as well.

Fixes #76022
Pull Request resolved: #75861
Approved by: https://github.com/thiagocrepaldi, https://github.com/garymm

(cherry picked from commit 166568d)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@garymm garymm garymm approved these changes

@thiagocrepaldi thiagocrepaldi thiagocrepaldi approved these changes

@BowenBao BowenBao Awaiting requested review from BowenBao

@shubhambhokare1 shubhambhokare1 Awaiting requested review from shubhambhokare1

Assignees

@thiagocrepaldi thiagocrepaldi

Labels

cla signed module: onnx Related to torch.onnx oncall: jit Add this issue/PR to JIT oncall triage queue open source release notes: onnx torch.onnx related changes that should show up in the release notes topic: improvements topic category

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[ONNX] torch.minimum() could not be exported successfully if given parameters have different dtypes.

6 participants

@fatcat-z @facebook-github-bot @garymm @thiagocrepaldi @pytorchmergebot @pytorchbot