Skip to content

Two fixes involving minimal builds - #17000

Merged
skottmckay merged 6 commits into
mainfrom
skottmckay/FixMinimalBuildIssues
Aug 23, 2023
Merged

Two fixes involving minimal builds#17000
skottmckay merged 6 commits into
mainfrom
skottmckay/FixMinimalBuildIssues

Conversation

@skottmckay

@skottmckay skottmckay commented Aug 4, 2023

Copy link
Copy Markdown
Contributor

Description

  • allocation planner was breaking if graph had no nodes

    • in this particular model a branch of an If node returned an outer scope value directly.
  • if model used non-tensor types and sparse tensors are disabled the call to IsSpareTensor causes an exception when prematurely terminates the code.

    • it's perfectly fine to check if a value is a sparse tensor when support for them is disabled. we just can't do anything with that OrtValue which is what the current ifdef's after the call to IsSparseTensor handle.

Motivation and Context

Fix model execution failure for partner with model that uses sequences in a minimal build with sparse tensors disabled.

- allocation planner was breaking if graph had no nodes
  - in this particular model a branch of an If node returned an outer scope value directly.

- if model used non-tensor types and sparse tensors are disabled the call to IsSpareTensor causes an exception when prematurely terminates the code.
  - it's perfectly fine to check if a value is a sparse tensor when support for them is disabled. we just can't do anything with that OrtValue which is what the current ifdef's after the call to IsSparseTensor handle.
@skottmckay
skottmckay requested a review from souptc August 4, 2023 06:46
- Update required ops for tests.
  - Manually updated required_ops.config
    - diffs with current version are massive and the config generation doesn't handle invalid model (we have one to test invalid external data).
  - updated required_ops_and_types.config as per readme
  - Not sure why there's a big gap in the configs. Unit tests for minimal build must not be using all the ORT format models which is understandable as testing a minimal build vs minimal build with operator reduction are two separate types of usage.
Comment thread onnxruntime/test/testdata/ort_github_issue_17000.py Fixed
Comment thread onnxruntime/test/testdata/ort_github_issue_17000.py Fixed
Comment thread onnxruntime/test/testdata/ort_github_issue_17000.py Fixed
Comment thread onnxruntime/test/testdata/ort_github_issue_17000.py Fixed
Comment thread onnxruntime/test/testdata/ort_github_issue_17000.py Fixed
@skottmckay
skottmckay requested a review from edgchen1 August 22, 2023 21:59
Comment thread onnxruntime/test/testdata/ort_github_issue_17000.py Fixed
Comment thread onnxruntime/test/testdata/ort_github_issue_17000.py Fixed
Comment thread onnxruntime/test/testdata/ort_github_issue_17000.py Fixed
Comment thread onnxruntime/test/testdata/ort_github_issue_17000.py Fixed
edgchen1
edgchen1 previously approved these changes Aug 22, 2023
Comment thread onnxruntime/test/framework/ort_model_only_test.cc Outdated
Comment thread onnxruntime/test/framework/ort_model_only_test.cc Outdated
Comment thread onnxruntime/test/testdata/ort_github_issue_17000.py
@skottmckay
skottmckay merged commit b3cb775 into main Aug 23, 2023
@skottmckay
skottmckay deleted the skottmckay/FixMinimalBuildIssues branch August 23, 2023 06:01
@faxu faxu added the triage:approved Approved for cherrypicks for release label Aug 24, 2023
Lafi7e pushed a commit that referenced this pull request Aug 28, 2023
### Description
<!-- Describe your changes. -->
- allocation planner was breaking if graph had no nodes
- in this particular model a branch of an If node returned an outer
scope value directly.

- if model used non-tensor types and sparse tensors are disabled the
call to IsSpareTensor causes an exception when prematurely terminates
the code.
- it's perfectly fine to check if a value is a sparse tensor when
support for them is disabled. we just can't do anything with that
OrtValue which is what the current ifdef's after the call to
IsSparseTensor handle.




### Motivation and Context
<!-- - Why is this change required? What problem does it solve?
- If it fixes an open issue, please link to the issue here. -->
Fix model execution failure for partner with model that uses sequences
in a minimal build with sparse tensors disabled.
snnn pushed a commit that referenced this pull request Aug 28, 2023
kleiti pushed a commit to kleiti/onnxruntime that referenced this pull request Mar 22, 2024
### Description
<!-- Describe your changes. -->
- allocation planner was breaking if graph had no nodes
- in this particular model a branch of an If node returned an outer
scope value directly.

- if model used non-tensor types and sparse tensors are disabled the
call to IsSpareTensor causes an exception when prematurely terminates
the code.
- it's perfectly fine to check if a value is a sparse tensor when
support for them is disabled. we just can't do anything with that
OrtValue which is what the current ifdef's after the call to
IsSparseTensor handle.




### Motivation and Context
<!-- - Why is this change required? What problem does it solve?
- If it fixes an open issue, please link to the issue here. -->
Fix model execution failure for partner with model that uses sequences
in a minimal build with sparse tensors disabled.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

triage:approved Approved for cherrypicks for release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants