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

Refactor train index and create index from template APIs in JNI layer #1918

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

Conversation

junqiu-lei
Copy link
Member

@junqiu-lei junqiu-lei commented Aug 1, 2024

Description

Follow up from #1784, this PR refactors CreateIndexFromTemplate and TrainIndex APIs within JNI layer.

  • Moved faiss lib train related function calls to their respective service classes.
  • Cleaned up and removed redundant code in faiss wrapper.

Related Issues

Closes #1846

Check List

  • New functionality includes testing.
  • New functionality has been documented.
  • API changes companion pull request created.
  • Commits are signed per the DCO using --signoff.
  • Public documentation issue/PR created.

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.

@junqiu-lei junqiu-lei added Refactoring Improve the design, structure, and implementation while preserving its functionality backport 2.x v2.17.0 labels Aug 1, 2024
@junqiu-lei junqiu-lei self-assigned this Aug 1, 2024
@jmazanec15
Copy link
Member

@junqiu-lei Could you provide context around what changes you are making and why? Itll help for review

@junqiu-lei
Copy link
Member Author

@junqiu-lei Could you provide context around what changes you are making and why? Itll help for review

Sure, updated in PR description.

@jmazanec15
Copy link
Member

@junqiu-lei Did ./bin/jni_test pass on your local?

@junqiu-lei
Copy link
Member Author

@junqiu-lei Did ./bin/jni_test pass on your local?

Yes, here is the results:

[==========] 74 tests from 39 test suites ran. (492 ms total)
[  PASSED  ] 74 tests.
(base) junqiu@6c7e67cc15a1 jni % git status
On branch refactor-jni-train
Your branch is up to date with 'junqiu/refactor-jni-train'.

Signed-off-by: Junqiu Lei <junqiu@amazon.com>
Signed-off-by: Junqiu Lei <junqiu@amazon.com>
@jmazanec15
Copy link
Member

@junqiu-lei are there next steps on this one?

@jmazanec15
Copy link
Member

@junqiu-lei are you still working on this one?

@junqiu-lei
Copy link
Member Author

@junqiu-lei are you still working on this one?

Yes, sorry for late reply, there was some integ failures when I raised out the refactor CR, will resume looking it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Refactoring Improve the design, structure, and implementation while preserving its functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor train model method in JNI
3 participants