|
| 1 | +# Contributing rules for AI agents (Claude, Codex, …) |
| 2 | + |
| 3 | +These are the house rules for any LLM-driven contribution to `tensordict`. They |
| 4 | +sit on top of [`CONTRIBUTING.md`](CONTRIBUTING.md), which still applies. When the |
| 5 | +two disagree, this file wins for AI-generated changes. |
| 6 | + |
| 7 | +Read this end-to-end before editing anything. |
| 8 | + |
| 9 | +## 1. Imports |
| 10 | + |
| 11 | +- **No local (function/method-level) imports.** Module-top imports only. |
| 12 | +- Two exceptions, and only two: |
| 13 | + - **Optional dependencies.** Gate them with a module-level |
| 14 | + `_has_<name> = importlib.util.find_spec("<name>") is not None`, then import |
| 15 | + the lib lazily inside the function — or, preferred, cache it on |
| 16 | + `self._<name>` the first time it is needed so subsequent calls don't re-run |
| 17 | + `import`. |
| 18 | + - **Genuine circular imports.** Before deferring, try |
| 19 | + `from typing import TYPE_CHECKING` with a guarded import — that handles the |
| 20 | + type-annotation case without paying a runtime cost. |
| 21 | +- **No wildcard imports** (`from x import *`). Remove them when you see them. |
| 22 | +- **`from __future__ import annotations`** at the top of every new `.py` file. |
| 23 | + |
| 24 | +## 2. Cross-version compatibility |
| 25 | + |
| 26 | +- Use `implement_for` from `pyvers` to dispatch on dependency versions — |
| 27 | + torch, numpy, etc. Do not hand-roll `if torch.__version__ >= …` branches. |
| 28 | + |
| 29 | + ```python |
| 30 | + from pyvers import implement_for |
| 31 | + ``` |
| 32 | + |
| 33 | +## 3. Logging, printing, timing |
| 34 | + |
| 35 | +- **Never use `print()`** in library code. Use the package logger: |
| 36 | + ```python |
| 37 | + from tensordict.utils import logger as tensordict_logger |
| 38 | + tensordict_logger.info("…") |
| 39 | + ``` |
| 40 | +- **For timing**, use `tensordict.utils.timeit` — never ad-hoc `time.time()` |
| 41 | + blocks. |
| 42 | + |
| 43 | +## 4. Container model |
| 44 | + |
| 45 | +`tensordict` *is* the container library, so the rule isn't "use TensorDict", |
| 46 | +it's "don't fork the abstraction": |
| 47 | + |
| 48 | +- New container-like behavior should extend `TensorDictBase` / `TensorDict` / |
| 49 | + `LazyStackedTensorDict` / `PersistentTensorDict` / `Tensorclass` / |
| 50 | + `TypedTensorDict`, not introduce parallel dict-like types. |
| 51 | +- Stick to `NestedKey` semantics for keying; don't invent a parallel key |
| 52 | + representation. |
| 53 | +- New tensorclass-style containers follow the existing `tensorclass` / |
| 54 | + `TypedTensorDict` patterns — match them rather than rolling your own. |
| 55 | + |
| 56 | +## 5. `torch.compile` / cudagraphs friendliness |
| 57 | + |
| 58 | +`tensordict` sits on the hot path of `torch.compile`-d code in many downstream |
| 59 | +projects, so this matters more here than in most libraries. Not mandatory, |
| 60 | +**strongly encouraged**: |
| 61 | + |
| 62 | +- Prefer `torch.where(...)` and masking over Python-level `if`/`else` on tensor |
| 63 | + values. |
| 64 | +- Avoid data-dependent shapes and `.item()` calls on hot paths. |
| 65 | +- Keep tensor dtypes/devices stable across calls. |
| 66 | + |
| 67 | +If a component touches a hot path (core `TensorDict` ops, key handling, batched |
| 68 | +ops, `nn` modules, memmap / store), please verify it under `torch.compile` and, |
| 69 | +where reasonable, under cudagraphs. `test/test_compile.py` and |
| 70 | +`benchmarks/compile/` are the existing baselines — extend them rather than |
| 71 | +inventing a new harness. |
| 72 | + |
| 73 | +## 6. Tests |
| 74 | + |
| 75 | +- **Every new public class / function needs tests.** |
| 76 | +- **Do not create new test files** when an existing one already covers the |
| 77 | + module/area — extend the existing file (`test/test_tensordict.py`, |
| 78 | + `test/test_nn.py`, `test/test_tensorclass.py`, `test/test_compile.py`, |
| 79 | + `test/test_memmap.py`, `test/test_store.py`, …). New test files should be |
| 80 | + reserved for genuinely new areas of the library. |
| 81 | + |
| 82 | +## 7. Documentation |
| 83 | + |
| 84 | +- **Every new public class / function must be referenced** in the appropriate |
| 85 | + `docs/source/reference/*.rst` page (`td.rst`, `tc.rst`, `nn.rst`, |
| 86 | + `ttd.rst`). PRs that add a class but skip the `.rst` entry will be sent |
| 87 | + back. |
| 88 | +- **Docstrings**: Sphinx-style (`Args:`, `Returns:`, `Examples:`) with a |
| 89 | + runnable `>>> …` example for every new public class. |
| 90 | +- **Paper references**: if a feature is inspired by a paper, include the |
| 91 | + arXiv link (and a short citation) in the class docstring. |
| 92 | +- **No emojis** anywhere — code, docstrings, comments, commit messages, PR |
| 93 | + bodies. |
| 94 | + |
| 95 | +## 8. Tutorials |
| 96 | + |
| 97 | +New "headline" features (a new container type, a major new `nn` component, |
| 98 | +a new storage backend, etc.) should ship a tutorial under |
| 99 | +`tutorials/sphinx_tuto/`, or extend an existing one. Take inspiration from the |
| 100 | +tutorials already in the repo. Tutorials are **Sphinx-first**, not |
| 101 | +script-first: |
| 102 | + |
| 103 | +- Use `# regular prose comments` for explanation, **not** `print("…")`. |
| 104 | +- Structure should mirror existing tutos, including (names can be rephrased): |
| 105 | + - a **"What you will learn"** section near the top, |
| 106 | + - a **"Conclusion"** section, |
| 107 | + - a **"Further reading"** section. |
| 108 | + |
| 109 | +## 9. Benchmarks |
| 110 | + |
| 111 | +If a change is performance-relevant — anything on a hot path (core |
| 112 | +`TensorDict` ops, key handling, indexing/stacking, `nn` modules, memmap, |
| 113 | +store, compile-related code) — add or extend a benchmark under `benchmarks/`. |
| 114 | +"Performance-relevant" is the trigger; pure correctness fixes don't need one. |
| 115 | + |
| 116 | +## 10. Type hints |
| 117 | + |
| 118 | +- New public signatures should carry type hints. Internal helpers can skip |
| 119 | + them, but prefer to add them when convenient. |
| 120 | +- Hints are documentary (we don't enforce them with mypy in CI — see |
| 121 | + `CONTRIBUTING.md`), but they must be **accurate** — wrong hints are worse |
| 122 | + than no hints. |
| 123 | + |
| 124 | +## 11. Backwards compatibility & deprecations |
| 125 | + |
| 126 | +We give users **two minor releases** of warning before any breaking change. |
| 127 | +Concretely, if the next release is `0.X`: |
| 128 | + |
| 129 | +- Deprecate in `0.X`, |
| 130 | +- Default-value changes (if any) can land in `0.(X+1)`, |
| 131 | +- Final removal / behavior switch in `0.(X+2)`. |
| 132 | + |
| 133 | +Rules: |
| 134 | + |
| 135 | +- Use `DeprecationWarning` for API removals; `FutureWarning` for upcoming |
| 136 | + default-value changes that users can already see. |
| 137 | +- **Always state the schedule explicitly** in the warning message, naming the |
| 138 | + version where the change will happen, e.g.: |
| 139 | + > `TensorDict.foo` is deprecated and will be removed in v0.X+2. Use |
| 140 | + > `TensorDict.bar` instead. |
| 141 | +
|
| 142 | +## 12. PR labels |
| 143 | + |
| 144 | +Use a `[Tag]` prefix on the PR title. Canonical set seen in the repo: |
| 145 | + |
| 146 | +``` |
| 147 | +[BE] [Benchmark] [BugFix] [CI] [Compile] [Deprecation] [Distributed] |
| 148 | +[Doc] [Feature] [Minor] [Performance] [Quality] [Refactor] [Test] |
| 149 | +[Versioning] |
| 150 | +``` |
| 151 | + |
| 152 | +Pick the most specific one. `[Feature]` for new user-facing capability, |
| 153 | +`[BugFix]` for fixes, `[Doc]` for docs-only, `[CI]` for workflows, |
| 154 | +`[BE]` for backend / internal plumbing, `[Performance]` for perf work, |
| 155 | +`[Quality]` for lint/typing/cleanup, `[Refactor]` for behavior-preserving |
| 156 | +restructure, `[Compile]` for `torch.compile` / cudagraphs work, |
| 157 | +`[Distributed]` for distributed / multi-process plumbing, |
| 158 | +`[Deprecation]` for deprecation warnings, `[Versioning]` for version bumps. |
| 159 | + |
| 160 | +## 13. Commits |
| 161 | + |
| 162 | +No squash requirement. Make commits that read sensibly on their own; that's |
| 163 | +all. |
| 164 | + |
| 165 | +## 14. When in doubt |
| 166 | + |
| 167 | +Read a recently-merged PR in the same area and match the conventions there |
| 168 | +before inventing your own. |
0 commit comments