-
Notifications
You must be signed in to change notification settings - Fork 445
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
Auto train type detection (Extension for auto-config functionality) #2195
Auto train type detection (Extension for auto-config functionality) #2195
Conversation
In this PR introduced:
Currently to start Semi-SL training or Self-SL we should explicitly set training type argument otx train MobileNet-V3-large-1x --train-data-roots data/flower_photos/train --val-data-roots data/flower_photos/val --unlabeled-data-roots tests/assets/imagenet_dataset --train-type Semisupervised We can simplify this command line to: otx train MobileNet-V3-large-1x --train-data-roots data/flower_photos/train --val-data-roots data/flower_photos/val
Implementation: Finally, using auto-config functionality users can run Semi-SL and Self-SL in same way as for Supervised training: otx train --train-data-roots data/flower_photos/train However, to distinguish between auto_split functionality and Self-SL we should explicitly pass training_type. The related documentation with all of use cases will be added in the next PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @kprokofi ,
I think this is a great idea for a feature.
However, I have a concern about its use in API (Geti as well).
As you know, API also needs configuration.yaml
and template.yaml
inside our config.
But they don't use ConfigManager. For-example, Geti doesn't use a CLI, they use the API directly.
So Auto
in configuration.yaml
and template.yaml
can be a problem.
I think a different implementation should be proposed.
- Either recognize Auto in the API and modify it to change (Not in ConfigManager)
or - Revert (configuration.yaml and template.yaml) and use Auto detect only for the CLI. (The default is Incremental, but we suggest giving CLI users an option called
Auto
.)
I can think of at least two ways to do this.
Yes, I agree with Harim's comment. I think it's a nice feature, but there may be many things to consider including not only what Harim said but also other things. |
Sure, I will think today how to avoid changing default training_type. |
We need to add support for running training with only images when training type is Self-SL. This will be done in next PRs after investigation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the great feature. I left a few comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your work! I left some comments. please take a look
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the good feature, BTW, as you know, this kind of automation could raise unexpected errors and I have a little concern about this. However, it is not crucial and just a minor concern.
36e03a3
to
3761285
Compare
I found a problem with dataset detection format when using unlabeled data inside dataset folder. Also in imagenet format this folder will be detected as additional label class, so, unfortunately we can't omit path to unlabeled images in CLI |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, Now we just need to fix it so that the test passes.
tests are passed |
@harimkang @sungmanc could you approve one more time? |
Summary
How to test
Checklist
License
Feel free to contact the maintainers if that's a concern.