support new args.num_classes for custom training, robustly and with full back-compat

[lessw2020]

This PR attempts to cleanly integrate a new args.num_classes support for custom datasets while keeping full backwards compatibility.
1 - a new args.num_classes is made in main.py, with a default of 20 for backward compat with previous code in detr::build(args) which defaulted to 20.

2 - detr::build(args) is updated to safely check for the presence of a args.num_classes. If found that num_classes is used.
If not present, num_classes is set to 20, as before.

{ I wrappered this check for args.num_classes in a try/except block because likely people already have modified main.py scripts (I did) that won't have args.num_classes present.
I tested and found that checking for args.num_classes is None, my first instinct, will throw an exception if args.num_classes does not exist, hence the try/except block is required.. }

3 - datasets coco and coco_panoptic are checked via args.dataset_file as previously, and num_classes is over-ridden if so and set as before.

[alcinos]

Hi @lessw2020 ,

Thanks for the PR. On the surface it looks good, however from a design perspective I wonder if it's a good idea to have the number of classes as an extra parameters.
I feel like it would be cleaner to make it a property of the dataset, and just use dataset.num_classes() where appropriate. Any thoughts on this @fmassa?

[fmassa]

Thanks for the PR @lessw2020 !

I agree with @alcinos that having a num_classes method in the dataset would be nice, but I think it would be a bit complicated / misleading to implement correctly because of a few reasons:

• the meaning of num_classes would actually mean max_class_id, as we do not do any remapping in the dataset
• users with arbitrary class ids would probably require a remapping of the ids otherwise they will have very large number of "classes", which requires more changes in the evaluation code

@lessw2020 how are you using the current dataset, did you implement your own copy of coco.py or are you just feeding a different input path to the json?

Defining how we want users to use custom datasets is IMO the first thing we should do here.

[lessw2020]

Hi @fmassa and @alcinos,
Thanks for the feedback and insight!
For my own training, I modified my own class based on coco.py with some additional dictionaries to support the class id remapping.
I agree that it would be ideal to define how users should create/use their custom datasets.
I was starting to work on a generic dataset class for the colab, to see if there was a way to create an elegant DETR base dataset class that users could then just customize to get going.

One other minor but related point - I've seen a number of people thinking they have to add an additional +1 to their num_classes in order to be compat with DETR's 'no object' class vs DETR auto-handles that:

detr.py, line 37 = self.class_embed = nn.Linear(hidden_dim, num_classes + 1)
Thus having a default custom dataset class or defining how users should setup their custom datasets might also eliminate that confusion.

Two example dataset classes I've seen (and leveraged from) are rwightman and signatrix:
https://github.com/signatrix/efficientdet/blob/master/src/dataset.py
https://github.com/rwightman/efficientdet-pytorch/blob/master/data/dataset.py

and lgvaz has more of a base abstraction concept here:
https://github.com/lgvaz/mantisshrimp/blob/master/mantisshrimp/metrics/coco_metric/coco_utils.py

Anyway, definitely agree that if you are able to provide guidelines on how you want users to use custom datasets that would be a big help and supercede the args.num_classes issue here.

I also think ideal if you have a base custom dataset class people can leverage off off of / inherit from directly, to let users quickly get going with training and with minimal friction which would implicitly guide how users use custom datasets.

Thanks for the consideration re; supporting custom datasets!

[fmassa]

I think once we provide a Detectron2-compatible wrapper around DETR (which should be soon), defining a custom dataset will be delegated to Detectron2 abstractions, so that we won't need to worry about it anymore.
Using Detectron2 brings additional overhead, but makes a number of things easier for the user.
So I think it might be the easiest route -- they can either use the simpler codebase of DETR, or rely on the more feature-complete abstractions from Detectron2.

Let me know what you think

[lessw2020]

Hi @fmassa - having detectron2 integration will be a big plus, but also seems like a lot of unique aspects of DETR might not be exposed well there?

DETR is completely unique in terms of core architecture, and how it trains (bs=2 etc), evaluates (eos_coeff, num_queries), etc. and I like the focus on light and efficient code that seems to be effused into DETR codebase.

Thus, I would still prefer to see DETR with it's own minimal, lightweight support for custom training and then users can work in either Detectron2 (more simple use) or DETR (lightweight but more customized/specialized) as works best for their use case.

I don't think that much net work is needed to do have that in basic form - support num_classes, have a base coco style class to inherit from or copy-modify, and maybe some tweaking tips for training ala coco eval and visualizations (example the attention decoder heatmaps that are unique to DETR)?

Anyway, I think with all of DETR's unique arch and features, it still deserves it's own lightweight custom support available.
I also think a lot of these unique issues will have to be addressed even in detectron2 b/c anyone trying to customize and train in detectron2 or custom lightweight base, will need all that same knowledge about how DETR is unique, doesn't train with batch size 32, potentially what is eos_coeff, would like to have DETR's unique heatmaps, etc.

I'm also happy to try and contribute to support basic custom training btw, not just asking you guys to do all the extra work :)

[woctezuma]

Rather than "number of classes in your dataset", maybe specify that:

• it is the max_class_id (actually, maybe wrong),
• or is it max_class_id+1? Confusion about num_classes #108 (comment)