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.

Sign up for GitHub

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

Jump to bottom

OTX decoupling: deployment #345

Merged
23 commits merged into from Mar 12, 2024
Merged

OTX decoupling: deployment #345

23 commits merged into from Mar 12, 2024

Conversation

igor-davidyuk
Copy link
Collaborator

@igor-davidyuk igor-davidyuk commented Feb 28, 2024 •
edited

TODO:

  • Make sure label number to name mapping is correct
  • Double check all the docstrings to remove OTX where it is mentioned
  • Add unit tests

@igor-davidyuk igor-davidyuk force-pushed the otx-decoupling branch 2 times, most recently from 208abb5 to db859a0 Compare March 6, 2024 11:52
igor-davidyuk and others added 8 commits March 6, 2024 12:53
@igor-davidyuk
introduce a postprocessing package
b5b9a91 
also group explainability actions together

Signed-off-by: Igor Davidyuk <igor.davidyuk@intel.com>
@igor-davidyuk
move postprocessing to the deployed_model module
5f5602b 
Signed-off-by: Igor Davidyuk <igor.davidyuk@intel.com>
@igor-davidyuk
push prediction to annotation converter to postprocessing module
c8a55e4 
Signed-off-by: Igor Davidyuk <igor.davidyuk@intel.com>
@igor-davidyuk
converters without otx
db859a0 
Signed-off-by: Igor Davidyuk <igor.davidyuk@intel.com>
@ljcornel @igor-davidyuk
Make datasets optional in Project data model
d6012dc
@ljcornel @igor-davidyuk
Fix token auth error handling
8011b16
@igor-davidyuk
otx not used for models from 1.15
cf8b761 
Signed-off-by: Igor Davidyuk <igor.davidyuk@intel.com>
@igor-davidyuk
replase match case with ifs
84dd934 
Signed-off-by: Igor Davidyuk <igor.davidyuk@intel.com>
@igor-davidyuk igor-davidyuk marked this pull request as ready for review March 7, 2024 18:32
@igor-davidyuk
add legacy converter support
e09634e 
Signed-off-by: Igor Davidyuk <igor.davidyuk@intel.com>
@igor-davidyuk
fix typing
fda809c 
Signed-off-by: Igor Davidyuk <igor.davidyuk@intel.com>
@igor-davidyuk
fix typing
b666533 
Signed-off-by: Igor Davidyuk <igor.davidyuk@intel.com>
@igor-davidyuk
fix docstrings
b8779da 
Signed-off-by: Igor Davidyuk <igor.davidyuk@intel.com>
@igor-davidyuk
typing fix
5f3f69d 
Signed-off-by: Igor Davidyuk <igor.davidyuk@intel.com>
@igor-davidyuk
fix anomaly converter for otx 1.4
8ae644b 
Signed-off-by: Igor Davidyuk <igor.davidyuk@intel.com>
@igor-davidyuk
fix anomaly label assignment
fed6ad1 
Signed-off-by: Igor Davidyuk <igor.davidyuk@intel.com>
@igor-davidyuk
converter tests
8c7827b 
Signed-off-by: Igor Davidyuk <igor.davidyuk@intel.com>
@igor-davidyuk
add doc strings to packages
e098b2d 
Signed-off-by: Igor Davidyuk <igor.davidyuk@intel.com>
@igor-davidyuk
fix typing
2e6e995 
Signed-off-by: Igor Davidyuk <igor.davidyuk@intel.com>
@igor-davidyuk
label timestamp fix
3d86dd1 
Signed-off-by: Igor Davidyuk <igor.davidyuk@intel.com>
@igor-davidyuk
Merge branch 'main' into otx-decoupling
c4ce69f
ljcornel
ljcornel reviewed Mar 11, 2024
View reviewed changes
Copy link
Collaborator

@ljcornel ljcornel left a comment

Choose a reason for hiding this comment

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

The PR looks really good as far as I'm concerned, great how it simplifies the Deployment code. I just have some minor comments on the code.

Should we also change the requirements.txt to require otx==1.4.4? I bumped it to otx==1.5 last week but don't we need 1.4.4 for this to work with Geti v1.8?

geti_sdk/deployment/deployed_model.py Outdated Show resolved Hide resolved
geti_sdk/deployment/deployed_model.py Outdated Show resolved Hide resolved
igor-davidyuk and others added 2 commits March 11, 2024 13:10
@igor-davidyuk @ljcornel
Update geti_sdk/deployment/deployed_model.py
fc17327 
Co-authored-by: Ludo Cornelissen <ludo.cornelissen@intel.com>
@igor-davidyuk @ljcornel
Update geti_sdk/deployment/deployed_model.py
17b25e2 
Co-authored-by: Ludo Cornelissen <ludo.cornelissen@intel.com>
@igor-davidyuk
Copy link
Collaborator Author

The PR looks really good as far as I'm concerned, great how it simplifies the Deployment code. I just have some minor comments on the code.

Should we also change the requirements.txt to require otx==1.4.4? I bumped it to otx==1.5 last week but don't we need 1.4.4 for this to work with Geti v1.8?

I cherry-picked a few commits from this PR
https://github.com/openvinotoolkit/geti-sdk/pull/339/files#diff-a1e273292c74165beb265f06575619a4ec7f9a041c3ff936bd55a569bd4b05ae
to fix the issue preventing us from connecting to GETi 1.15.

I also merged the main branch to this one to get tests passing.

But generally, I see this PR and PR#339 as mutually exclusive as they solve the same problem, that is supporting OTX1.5 models

@ljcornel ljcornel mentioned this pull request Mar 11, 2024
Merged
@ljcornel ljcornel closed this pull request by merging all changes into openvinotoolkit:main in 46caa58 Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ljcornel ljcornel ljcornel approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@igor-davidyuk @ljcornel