-
Notifications
You must be signed in to change notification settings - Fork 7
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
44 panoptic docs #94
44 panoptic docs #94
Conversation
alodataset/coco_panoptic_dataset.py
Outdated
@@ -44,6 +45,9 @@ class CocoPanopticDataset(BaseDataset, SplitMixin): | |||
Include masks labels in the output, by default True | |||
classes : list, optional | |||
List of classes to be filtered in the annotation reading process, by default None | |||
fix_classes_len : int, optional | |||
Fix to a specific number the number of classes, filling the rest with "N/A" value. | |||
Use when the number of model outputs does not match with the number of classes in the dataset, by default 250 |
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.
Why is the default value 250 ? To match the model ? If so, I guess the default value should be None.
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.
@thibo73800 Ok, I will change in #89
alodataset/coco_panoptic_dataset.py
Outdated
@@ -101,6 +111,23 @@ def __init__( | |||
items.append(self.items[i]) | |||
self.items = items | |||
|
|||
# Fix label_types |
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.
Can you give more details about what the following is doing ?
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.
Done
# import wandb | ||
|
||
|
||
class BaseMetricsCallback(pl.Callback): |
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.
Not sure BaseMetricsCallback
is descriptive enough about the kind of data handle by this class. Currently BaseMetricsCallback is used for Panoptic & mAp right ? But not for ANY type of metrics. Just Objects/instance based metrics. So i'm not sure which name could be the most appropriate but, probably not this one.
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.
@thibo73800 fix in #89
@thibo73800: For this branch, is required merge first #89 to see the docs changes only |
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 all the documentation, can you add detr_panoptic to the main readme along with a specific readme in the detr_panoptic folder ? (Following other models example)
@thibo73800 : I updated the Ready to merge |
New documentations:
Update documentation of:
And fix link by new DataConnectors modules (explained in #89 )
**Important note: Not merge until merge first #89 **
Closes #2 - #44 - #104