pytorch / vision

Datasets, Transforms and Models specific to Computer Vision
https://pytorch.org/vision
BSD 3-Clause "New" or "Revised" License
16.19k stars 6.95k forks source link

New Model Architectures - Implementation and Documentation Details #5319

Open jdsgomes opened 2 years ago

jdsgomes commented 2 years ago

šŸš€ The feature

When adding a new model architecture there are some design/implementation details and documentation requirements that need to be taken into account. This issue intents to track such details in a dynamic manner, as it is possible to change over time.

Motivation, pitch

New Model Architectures - Implementation Details

Model development and training steps

When developing a new model there are some details not to be missed:

Note that this list is not exhaustive and there are details here related to the code quality etc, but these are rules that apply in all PRs (see Contributing to TorchVision).

Once the model is implemented, you need to train the model using the reference scripts. For example, in order to train a classification resnet18 model you would:

  1. go to references/classification

  2. run the train command (for example torchrun --nproc_per_node=8 train.py --model resnet18)

After training the model, select the best checkpoint and estimate its accuracy with a batch size of 1 on a single GPU. This helps us get better measurements about the accuracy of the models and avoid variants introduced due to batch padding (read here for more details).

Finally, run the model test to generate expected model files for testing. Please include those generated files in the PR as well.:

EXPECTTEST_ACCEPT=1 pytest test/test_models.py -k {model_name}

Documentation and Pytorch Hub

Alternatives

No response

Additional context

No response

datumbox commented 2 years ago

I've pinned the issue for now but we should consider making use of the Wiki pages, which are better suited for this kind of content. PyTorch core uses them extensively, so it might be worth aligning.

NicolasHug commented 2 years ago

Are we expecting these guidelines to change often? Otherwise we might also consider just having this as a .md file in the repo (e.g. as part of CONTRIBUTING_MODELS.md) ?

jdsgomes commented 2 years ago

That is a good point, and has been discussed previously with @datumbox and also during this PR review. In short I think is fair so say that there are no strong feelings either way, but there were two main arguments to keep it in a ticket for now. First we didn't want to make the contribution guidelines too long, second the content can change. So I would still favour to keep it here for a while and if it seems stable enough we can move it to a .md file

datumbox commented 2 years ago

Things will eventually become stable. I think the biggest changes on this documentation will come from the following:

Once things stabilise we can move to md if you prefer.

yassineAlouini commented 2 years ago

Thanks for this great guide @jdsgomes. There is a small typo here: ConvNormActication (should be Activation)