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

add BAAI/bge-small-en-v1.5 Optimization #1634

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open

Conversation

xieofxie
Copy link
Contributor

@xieofxie xieofxie commented Feb 21, 2025

Describe your changes

Checklist before requesting a review

  • Add unit tests for this change.
  • Make sure all tests can pass.
  • Update documents if necessary.
  • Lint and apply fixes to your code by running lintrunner -a
  • Is this a user-facing change? If yes, give a description of this change to be included in the release notes.
  • Is this PR including examples changes? If yes, please remember to update example documentation in a follow-up PR.

(Optional) Issue link

"calibrate_method": "MinMax",
"quant_preprocess": true,
"prepare_qnn_config": true,
"op_types_to_quantize": [
Copy link
Contributor

@jambayk jambayk Feb 21, 2025

Choose a reason for hiding this comment

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

I have a dev branch where I introduce an option called op_type_to_exclude which is used to modify op_types_to_quantize and nodes_to_exclude.

"op_types_to_exclude": PassConfigParam(

Looks like it might be useful here too when it gets merged

Copy link
Contributor

Choose a reason for hiding this comment

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

otherwise, we need to know all of the op types present in the model.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

currently use append_first_op_types_to_quantize_list with nodes_to_exclude will do this. Will we also update this logic?

if run_config["append_first_op_types_to_quantize_list"]:

Copy link
Contributor

Choose a reason for hiding this comment

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

honestly, I am not sure why this option was added and if it is used for anything right now.

Not sure if we will touch this option and related logic but I plan to update the logic to be able to use op_types_to_exclude and nodes_to_exclude. The op_types_to_exclude has been very useful for me when I know I don't want to quantize all nodes for an op.

Copy link
Contributor

Choose a reason for hiding this comment

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

also created this PR in ort microsoft/onnxruntime#23779.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we could merge following the clip?

@xieofxie xieofxie marked this pull request as draft February 21, 2025 09:06
@xieofxie xieofxie marked this pull request as ready for review February 24, 2025 06:50
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