frozenleaves/pytorch
1
0
Fork 0
mirror of https://github.com/pytorch/pytorch.git synced 2025-10-20 21:14:14 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix typo

derpbuffalo
2024-09-30 13:36:30 +02:00
parent e87b46cde1
commit 4776c89925
1 changed files with 1 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
Code-review-values.md

@ -25,7 +25,7 @@ The purpose of this document is to describe PyTorch's “code review values”;
## **About testing**
* **Are there tests?** Tests should come in the same diff as the functionality that is added, as protection from the author forgetting to add them later. Tests are extra important for Python code, since otherwise there is there is no compile-time checked type system to automatically flush out what call sites are now wrong. If the diff fixes a bug, is there a test that exercises the bug: failed before and no longer fails? If the bug is not easy to test, did the diff author describe how they tested the change by hand?
* **Are there tests?** Tests should come in the same diff as the functionality that is added, as protection from the author forgetting to add them later. Tests are extra important for Python code, since otherwise there is no compile-time checked type system to automatically flush out what call sites are now wrong. If the diff fixes a bug, is there a test that exercises the bug: failed before and no longer fails? If the bug is not easy to test, did the diff author describe how they tested the change by hand?
* **Are the tests comprehensive?** Suppose you added an operator that takes some optional arguments. Do your tests test each of the optional arguments, individually and all together? If an argument takes either a number or a tuple, do you test with a tuple with differing values for its elements? Do you test it on both contiguous and non-contiguous inputs? (The standard test harness checks for some of these things, but at the end of the day it's jointly the responsibility of the author and the code reviewer to make sure that the harness in conjunction with the written tests are sufficient.)
* **For hard to test changes, did the author describe how they tested their change?** Suppose a diff was submitted to improve support for some GPU on OS X situation. We don't have this configuration in our continuous integration, and thus cannot conveniently test the change. In that case, the author should give some test plan / evidence that their change in fact works.
* **If the change is a performance optimization, did the diff author include a benchmark?** A simple timeit invocation showing change in wall clock improvement is much better than nothing at all. If you are optimizing, you have an obligation to show that your optimization also showed an improvement.