-
Notifications
You must be signed in to change notification settings - Fork 443
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
Enable hierarchical classification in MPA #1159
Conversation
…_extensions into mpa-instance-seg
…_extensions into mpa-instance-seg
…_extensions into mpa-instance-seg
…training_extensions into sc-intg-ins-seg
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.
I left a few comments.
Additionally,
- When I looked at the added sample data, I could look at the data sets that already exist, such as 'data/car_tree_bug'. Is there a reason to add an image directly? I think you just need to specify the path in the annotation file, what do you think? (The capacity may increase due to overlapping image data.)
- There seems to be a conflict in the submodule of the MPA. Please check.
external/model-preparation-algorithm/mpa_tasks/apis/classification/task.py
Outdated
Show resolved
Hide resolved
external/model-preparation-algorithm/mpa_tasks/apis/classification/task.py
Outdated
Show resolved
Hide resolved
external/model-preparation-algorithm/mpa_tasks/extensions/datasets/mpa_cls_dataset.py
Outdated
Show resolved
Hide resolved
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.
Some code looks complicated. It would be good to divide it into functions, but it would be good to consider refactoring with SAMClassifier later 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.
LGTM
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
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.
But please note that we need refactoring in future PRs.
There are too many / long if-else blocks that should be extracted as method or subclassed.
This PR includes:
Summary