update contributing

This commit is contained in:
George Hotz
2023-12-19 12:19:14 -08:00
parent e477cc2f45
commit ac6ec936cd
2 changed files with 21 additions and 45 deletions

View File

@@ -1,33 +0,0 @@
# Are you ready to write high quality code?
The idea of tinygrad is to build a <5000 line library capable of training a wide variety of ML models at 80%+ max theoretical speed across a huge variety of hardware.
There is almost no boilerplate code anywhere in this library, and you should help keep it that way. If the code you are contributing to core tinygrad, in `tinygrad/`, isn't some of the highest quality code you've written in your life, either put in the effort to make it great, or don't bother. (other directories have a slightly more relaxed standard)
There is a linter, but it's not complete. Spend a little time reading the existing code to get a feel for the style.
I love PRs where I can look at them and just say, yes, this will improve the codebase and click merge. If you have an incomplete PR, feel free to post it as a draft.
As my operating systems professor taught me, code is written to be read by humans. We value readability over performance and line count, but low line count is often a good proxy for readability. However, any PRs that look like code golf will immediately be closed.
There are a few basic ways to contribute:
## Bug-fixes
These are the most straightforward. Discover a bug. Add a test to reproduce it. Write a clean fix. Submit a PR. Confirm CI passes.
## Conceptual Cleanups
This is some of the highest value work in tinygrad. If you realize two 50 line functions are basically the same thing, and you can merge them, amazing! Things that look confusing and are hard to follow are probably poorly written. If you can rewrite the code and be like, oh that's a ton simpler, by all means do so. Make sure you have good test coverage around what you are changing.
## Better Testing
Always welcome! Think about how robust and fast your tests are though. How likely is this test to catch a bug? Tests that run in CI go in `test/`, except for the ones in `test/external/`. We have a few things like fuzzers in there.
## Speed improvements
tinygrad is a JIT compiler, so speed improvements refer to both compile-time and runtime. Speed improvements to the python based compiler are welcome, but please include benchmarks and good tests around the things that you are changing. If you are sacrificing readability for speed, don't bother. Speed improvements to the generated code usually come from conceptual cleanups. Generated code improvements are probably the hardest thing to work on in tinygrad, since they must be done in a very generic way.
## Features
This is a trickier one. If there is a feature in PyTorch and numpy that you have actually seen people use, we probably want it. All new features must include good robust tests, and in general, matching the PyTorch API is good.

View File

@@ -136,24 +136,33 @@ print(y.grad.numpy()) # dz/dy
## Contributing
There has been a lot of interest in tinygrad lately. Here are some basic guidelines for contributing:
There has been a lot of interest in tinygrad lately. Following these guidelines will help your PR get accepted.
- Bug fixes are the best and always welcome! Like [this one](https://github.com/tinygrad/tinygrad/pull/421/files).
- If you don't understand the code you are changing, don't change it!
- All code golf PRs will be closed, but [conceptual cleanups](https://github.com/tinygrad/tinygrad/pull/372/files) are great.
- Features are welcome. Though if you are adding a feature, you need to include tests.
- Improving test coverage is great, with reliable non-brittle tests.
We'll start with what will get your PR closed with a pointer to this section:
Additional guidelines can be found in [CONTRIBUTING.md](/CONTRIBUTING.md).
- No code golf! While low line count is a guiding light of this project, anything that remotely looks like code golf will be closed. The true goal is reducing complexity and increasing readability, and deleting `\n`s does nothing to help with that.
- All docs and whitespace changes will be closed unless you are a well-known contributor. The people writing the docs should be those who know the codebase the absolute best. People who have not demonstrated that shouldn't be messing with docs. Whitespace changes are both useless *and* carry a risk of introducing bugs.
- Anything you claim is a "speedup" must be benchmarked. In general, the goal is simplicity, so even if your PR makes things marginally faster, you have to consider the tradeoff with maintainablity and readablity.
- In general, the code outside the core `tinygrad/` folder is not well tested, so unless the current code is there is broken, you shouldn't be changing it.
Now, what we want:
- Bug fixes (with a regression test) are great! This library isn't 1.0 yet, so if you stumble upon a bug, fix it, write a test, and submit a PR, this is valuable work.
- Solving bounties! tinygrad [offers cash bounties](https://docs.google.com/spreadsheets/d/1WKHbT-7KOgjEawq5h5Ic1qUWzpfAzuD_J06N1JwOCGs/edit?usp=sharing) for certain improvements to the library. All new code should be high quality and well tested.
- Features. However, if you are adding a feature, consider the line tradeoff. If it's 3 lines, there's less of a bar of usefulness it has to meet over something that's 30 or 300 lines. All features must have regression tests. In general with no other constraints, your feature's API should match torch or numpy.
- Refactors that are clear wins. In general, if your refactor isn't a clear win it will be closed. But some refactors are amazing! Think about readability in a deep core sense. A whitespace change or moving a few functions around is useless, but if you realize that two 100 line functions can actually use the same 110 line function with arguments while also improving readability, this is a big win.
- Tests/fuzzers. If you can add tests that are non brittle, they are welcome. We have some fuzzers in here too, and there's a plethora of bugs that can be found with them and by improving them. Finding bugs, even writing broken tests (that should pass) with `@unittest.expectedFailure` is great. This is how we make progress.
- Dead code removal from core `tinygrad/` folder. We don't care about the code in extra, but removing dead code from the core library is great. Less for new people to read and be confused by.
### Running tests
You should install the pre-commit hooks with `pre-commit install`. This will run the linter, mypy, and a subset of the tests on every commit.
For more examples on how to run the full test suite please refer to the [CI workflow](.github/workflows/test.yml).
Some examples:
Some examples of running tests locally:
```sh
python3 -m pip install -e '.[testing]'
python3 -m pytest
python3 -m pytest -v -k TestTrain
python3 ./test/models/test_train.py TestTrain.test_efficientnet
python3 -m pip install -e '.[testing]' # install extra deps for testing
python3 test/test_ops.py # just the ops tests
python3 -m pytest test/ # whole test suite
```