Skip to content
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

Provide kwargs to remove_shared_initializers #17539

Merged
merged 1 commit into from
Sep 18, 2023

Conversation

jambayk
Copy link
Contributor

@jambayk jambayk commented Sep 13, 2023

Description

Fixes a bug in get_shared_initializers where signature_cache1, signature_cache2 are passed as positional arguments to remove_shared_initializers but their positions don't match the function signature. So signature_cache1 is passed to min_elements and causes comparison error at line 907.

Pass the arguments as kwargs so that it doesn't rely on their positions.

Motivation and Context

Fixes the bug described above.

@petermcaughan
Copy link
Contributor

/azp run Linux CPU CI Pipeline,Linux CPU Minimal Build E2E CI Pipeline,Linux GPU CI Pipeline,Linux GPU TensorRT CI Pipeline,Linux OpenVINO CI Pipeline,Linux QNN CI Pipeline,MacOS CI Pipeline,ONNX Runtime Web CI Pipeline,Windows ARM64 QNN CI Pipeline,Windows CPU CI Pipeline,Windows GPU CI Pipeline,Windows GPU TensorRT CI Pipeline,onnxruntime-binary-size-checks-ci-pipeline,orttraining-linux-ci-pipeline,orttraining-linux-gpu-ci-pipeline,orttraining-ortmodule-distributed

@azure-pipelines
Copy link

You have several pipelines (over 10) configured to build pull requests in this repository. Specify which pipelines you would like to run by using /azp run [pipelines] command. You can specify multiple pipelines using a comma separated list.

@petermcaughan
Copy link
Contributor

/azp run Linux CPU CI Pipeline, Linux CPU Minimal Build E2E CI Pipeline, Linux GPU CI Pipeline, Linux GPU TensorRT CI Pipeline, Linux Nuphar CI Pipeline, Linux OpenVINO CI Pipeline, MacOS CI Pipeline, Windows CPU CI Pipeline, Windows GPU CI Pipeline, Windows GPU TensorRT CI Pipeline

@petermcaughan
Copy link
Contributor

/azp run onnxruntime-python-checks-ci-pipeline, orttraining-amd-gpu-ci-pipeline, orttraining-linux-ci-pipeline, orttraining-linux-gpu-ci-pipeline, orttraining-ortmodule-distributed, ​ONNX Runtime Web CI Pipeline, onnxruntime-binary-size-checks-ci-pipeline

@azure-pipelines
Copy link

Azure Pipelines successfully started running 9 pipeline(s).

@azure-pipelines
Copy link

Azure Pipelines successfully started running 5 pipeline(s).

@petermcaughan
Copy link
Contributor

/azp run Linux QNN CI Pipeline,ONNX Runtime Web CI Pipeline,Windows ARM64 QNN CI Pipeline

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@petermcaughan
Copy link
Contributor

/azp run Windows x64 QNN CI Pipeline

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@petermcaughan petermcaughan merged commit f969e7f into microsoft:main Sep 18, 2023
@jambayk jambayk deleted the jambayk/initializer-bug-fix branch September 18, 2023 23:41
tianleiwu pushed a commit that referenced this pull request Oct 31, 2023
### Description
Fixes a bug in `get_shared_initializers` where `signature_cache1,
signature_cache2` are passed as positional arguments to
`remove_shared_initializers` but their positions don't match the
function signature. So `signature_cache1` is passed to `min_elements`
and causes comparison error at line 907.

Pass the arguments as kwargs so that it doesn't rely on their positions.


### 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. -->
Fixes the bug described above.
tianleiwu pushed a commit that referenced this pull request Oct 31, 2023
Fixes a bug in `get_shared_initializers` where `signature_cache1,
signature_cache2` are passed as positional arguments to
`remove_shared_initializers` but their positions don't match the
function signature. So `signature_cache1` is passed to `min_elements`
and causes comparison error at line 907.

Pass the arguments as kwargs so that it doesn't rely on their positions.

<!-- - Why is this change required? What problem does it solve?
- If it fixes an open issue, please link to the issue here. -->
Fixes the bug described above.
kleiti pushed a commit to kleiti/onnxruntime that referenced this pull request Mar 22, 2024
### Description
Fixes a bug in `get_shared_initializers` where `signature_cache1,
signature_cache2` are passed as positional arguments to
`remove_shared_initializers` but their positions don't match the
function signature. So `signature_cache1` is passed to `min_elements`
and causes comparison error at line 907.

Pass the arguments as kwargs so that it doesn't rely on their positions.


### 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. -->
Fixes the bug described above.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants