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

Added Self-SL adapter for classification to handle case when only images for train data provided + Documentation update #2219

Merged
merged 5 commits into from
Jun 12, 2023

Conversation

kprokofi
Copy link
Contributor

@kprokofi kprokofi commented Jun 5, 2023

Summary

How to test

Checklist

  • I have added unit tests to cover my changes.​
  • I have added integration tests to cover my changes.​
  • I have added e2e tests for validation.
  • I have added the description of my changes into CHANGELOG in my target branch (e.g., CHANGELOG in develop).​
  • I have updated the documentation in my target branch accordingly (e.g., documentation in develop).
  • I have linked related issues.

License

  • I submit my code changes under the same Apache License that covers the project.
    Feel free to contact the maintainers if that's a concern.
  • I have updated the license header for each file (see an example below).
# Copyright (C) 2023 Intel Corporation
# SPDX-License-Identifier: Apache-2.0

@kprokofi kprokofi requested a review from a team as a code owner June 5, 2023 17:06
@github-actions github-actions bot added the CLI Any changes in OTE CLI label Jun 5, 2023
@kprokofi
Copy link
Contributor Author

kprokofi commented Jun 5, 2023

The idea is to add fake label to "image dir" dataset format for classification in case of Self-SL when no annotations are provided. This will let us to construct OTX dataset and proceed with Byol unsupervised training providing just folder with any images

@github-actions github-actions bot added the DOC Improvements or additions to documentation label Jun 6, 2023
@kprokofi kprokofi changed the title Added Self-SL adapter for classification to handle case when only images for train data provided Added Self-SL adapter for classification to handle case when only images for train data provided + Documentation update Jun 6, 2023
Copy link
Contributor

@sungmanc sungmanc left a comment

Choose a reason for hiding this comment

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

Overall, LGTM. BTW, how about setting the default training_type as incremental?
And triggering auto mode by inserting --train-type auto?

I know this is just my preference, so this is a minor comment.
In my opinion, middle~high level developer assumes the supervised manner is default.

Copy link
Contributor

@sungchul2 sungchul2 left a comment

Choose a reason for hiding this comment

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

LGTM, but I have a question.
Do users should set only folder with images to use self-sl?
What about if users want self-sl w/ class folders?

@kprokofi
Copy link
Contributor Author

kprokofi commented Jun 7, 2023

LGTM, but I have a question. Do users should set only folder with images to use self-sl? What about if users want self-sl w/ class folders?

Sure, users can run Self-SL with image folder dataset as before. However, in that case we need to add --train-type option as I described in docs

@github-actions github-actions bot added the TEST Any changes in tests label Jun 7, 2023
@kprokofi
Copy link
Contributor Author

kprokofi commented Jun 7, 2023

Overall, LGTM. BTW, how about setting the default training_type as incremental? And triggering auto mode by inserting --train-type auto?

I know this is just my preference, so this is a minor comment. In my opinion, middle~high level developer assumes the supervised manner is default.

Setting train_type as None by default is solution how to distinguish between actual incremental passed by user in CLI and just default value. As we discussed on the meeting it is the most simple and working solution for that.
IMO, setting new value for train_type "--auto" doesn't make sense since the main purpose of this feature is to set training type automatically without typing "--train type"

Now, there is no overlap with corner cases and using auto train type detection is safe. I documented also one corner case, when we pass image folder format to SelfSL task instead of images only. In that case we have overlap with auto split for incremental. I also added this note in documentation both in explanation and tutorial sections.

@sungchul2
Copy link
Contributor

@kprokofi Thanks for answering. Let's take a look at test issues.

Copy link
Contributor

@sungmanc sungmanc 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 the explanation, before merging, we need to resolve test failures.

@kprokofi
Copy link
Contributor Author

I think it was a CI error
Locally everything is fine with OV unit tests
image

@sungmanc sungmanc merged commit 6b045da into openvinotoolkit:develop Jun 12, 2023
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLI Any changes in OTE CLI DOC Improvements or additions to documentation TEST Any changes in tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants