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

Fix make_model_config_json function #188

Merged

Conversation

thanawan-atc
Copy link
Contributor

@thanawan-atc thanawan-atc commented Jul 5, 2023

Description

Fix make_model_config_json function so that we can trace the model without having to create the model config file by ourselves.

Have been tested on tracing the following models in TorchScript and ONNX format:
sentence-transformers/msmarco-distilbert-base-tas-b
sentence-transformers/multi-qa-MiniLM-L6-cos-v1
sentence-transformers/all-MiniLM-L12-v2
sentence-transformers/multi-qa-mpnet-base-dot-v1

Issues Resolved

  • Allow user to specify the model_format
  • Enable it to scrape pooling_mode from Huggingface
  • Enable it to scrape normalize_result from Huggingface
  • Fix a bug in scraping embedding_dimension

Check List

  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>
Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>
Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>
@codecov
Copy link

codecov bot commented Jul 6, 2023

Codecov Report

Merging #188 (dcb0755) into main (976a512) will increase coverage by 0.15%.
The diff coverage is 92.30%.

@@            Coverage Diff             @@
##             main     #188      +/-   ##
==========================================
+ Coverage   90.81%   90.96%   +0.15%     
==========================================
  Files          37       37              
  Lines        4050     4073      +23     
==========================================
+ Hits         3678     3705      +27     
+ Misses        372      368       -4     
Impacted Files Coverage Δ
...search_py_ml/ml_models/sentencetransformermodel.py 70.55% <92.30%> (+2.90%) ⬆️

Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>
Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>
Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>
Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>
Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>
Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>
Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>
Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>
Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>
Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>
Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>
Copy link
Collaborator

@dhrubo-os dhrubo-os left a comment

Choose a reason for hiding this comment

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

Thanks for adding all those tests.

@dhrubo-os dhrubo-os merged commit 13149e8 into opensearch-project:main Jul 7, 2023
13 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jul 7, 2023
* Improve make_model_config_json function

Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>

* Fix linting issues

Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>

* Add CHANGELOG.md

Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>

* Add unittest

Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>

* Correct linting

Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>

* Correct linting

Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>

* Fix bug

Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>

* Minor Edit + Add more tests

Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>

* Fix test

Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>

* Correct typos

Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>

* Increase test coverage

Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>

* Increase test coverage (2)

Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>

* Increase test coverage (3)

Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>

* Remove redundant line

Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>

---------

Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>
(cherry picked from commit 13149e8)
dhrubo-os pushed a commit that referenced this pull request Jul 10, 2023
* Improve make_model_config_json function

Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>

* Fix linting issues

Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>

* Add CHANGELOG.md

Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>

* Add unittest

Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>

* Correct linting

Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>

* Correct linting

Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>

* Fix bug

Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>

* Minor Edit + Add more tests

Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>

* Fix test

Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>

* Correct typos

Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>

* Increase test coverage

Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>

* Increase test coverage (2)

Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>

* Increase test coverage (3)

Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>

* Remove redundant line

Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>

---------

Signed-off-by: Thanawan Atchariyachanvanit <latchari@amazon.com>
(cherry picked from commit 13149e8)

Co-authored-by: Thanawan Atchariyachanvanit <latchari@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants