Bonnes Pratiques de Contribution au Code TensorRT-LLM

Processus de Contribution

1. Flux de Travail du Développeur

1. Commit les modifications. Ne jamais commiter avec une adresse email NVIDIA interne (<user>@nvidia.com) !
2. Push les modifications vers une branche du fork personnel :

   git push -u <user> <local-branch>:<remote-branch>

3. Créer une PR depuis la branche du fork vers upstream (généralement en ciblant main).

2. Directives de Codage

Le style de codage TRT-LLM est défini dans CODING_GUIDELINES.md. Points clés :

C++: Style Allman, indentation 4 espaces, limite de 120 caractères par ligne, camelCase pour les variables/méthodes, PascalCase pour les types, préfixe m pour les variables membres, préfixe k pour les constantes, Doxygen pour la documentation des API, pointeurs intelligents au lieu de pointeurs bruts, static_cast au lieu de reinterpret_cast, pas de casts de style C.

Python: snake_case pour les fichiers/fonctions/variables, PascalCase pour les classes, UPPER_SNAKE_CASE pour les constantes, indentation 4 espaces, docstrings de style Google, clauses except restreintes, StrictBaseModel de Pydantic pour les classes de configuration destinées aux utilisateurs (pas de __init__ personnalisé).

3. Configuration de Pre-commit

pip install pre-commit
pre-commit install

Pre-commit s'exécute automatiquement à chaque git commit. Les hooks incluent : isort, yapf, autoflake, clang-format, cmake-format, codespell, ruff, ruff-format, mdformat, et autres. Si les hooks modifient des fichiers, les ajouter au staging et les commiter à nouveau.

4. Signature DCO (Obligatoire)

Tous les commits doivent être signés pour certifier la contribution selon le Developer Certificate of Origin :

git commit -s -m "Add cool feature."

Ceci ajoute Signed-off-by: Your Name <your@email.com> au message du commit. Les PRs contenant des commits non signés ne seront pas acceptées.

IMPORTANT : Ne jamais signer les commits avec une adresse email NVIDIA interne (<user>@nvidia.com) !

Liste de Vérification Pré-Implémentation

Avant d'écrire du code, complétez ces étapes :

1. Examiner l'Infrastructure Existante

Chercher avant de construire. TRT-LLM est une grande base de code avec de nombreux composants réutilisables. Avant d'implémenter quelque chose à partir de zéro, cherchez les utilitaires existants :

# Avant d'écrire un nouveau calcul d'attention
grep -r "TrtllmAttention\|create_attention\|scaled_dot_product" tensorrt_llm/_torch/

# Avant d'écrire un nouveau helper compilé
grep -r "maybe_compile\|maybe_compiled_" tensorrt_llm/_torch/utils.py

# Avant d'écrire une RoPE personnalisée
grep -r "RotaryEmbedding\|rotary_emb\|rope" tensorrt_llm/_torch/modules/

# Avant d'écrire un nouveau schéma de gestion de cache
grep -r "mla_rope_append_paged_kv\|append_paged_kv" tensorrt_llm/_torch/

Tracer les méthodes forward existantes. Avant d'écrire une nouvelle méthode forward_*, lisez toutes les méthodes forward existantes dans la classe et comprenez ce que chacune fait. Souvent, une méthode existante implémente déjà le calcul dont vous avez besoin, et vous avez juste besoin de configurer le bon état (par exemple, créer un attribut, ajuster une garde) pour la dispatcher.

# Trouver toutes les méthodes forward dans une classe
grep -n "def forward" tensorrt_llm/_torch/modules/attention.py
# Puis LIRE chacune pour comprendre ce qu'elle fait

Leçon apprise : Sur la branche short-seq MHA (30 commits, ~250 lignes écrites puis supprimées), le calcul d'attention a subi 4 réécritures : boucle SDPA par séquence → SDPA avec pad_sequence → backend TrtllmAttention personnalisé → suppression en faveur du forward_context_default() déjà existant. L'approche finale était +10 lignes : une vérification de garde + dispatch à une méthode existante. De même, maybe_compiled_cat n'a été découvert qu'après qu'un wrapper @maybe_compile autonome ait été écrit puis supprimé.

Anti-pattern : Réimplémentation parallèle. Avant d'écrire une nouvelle méthode forward_*, tracez ce que font les méthodes forward existantes. La nouvelle méthode peut déjà être implémentée. Dans le cas de MLA, forward_context_short_mha a réimplémenté forward_context_default presque ligne par ligne avant d'être supprimée.

2. Vérifier les Dimensions de Parallélisation

Quand vous ajoutez un nouveau chemin de code, vérifiez la correction sous TOUS les modes de parallélisation :

Leçon apprise : Le chemin short-seq MHA supposait que tous les tokens étaient locaux, ce qui casse sous Context Parallelism. La garde cp_size == 1 a été ajoutée comme correctif dans un commit ultérieur au lieu de faire partie de la conception initiale.

3. Réfléchir à la Sémantique du Seuil/Garde

Quand vous gâtez un chemin de code avec un seuil :

• Qu'est-ce que le seuil mesure ? (métrique par séquence ? métrique du batch total ?)
• Avec quoi évolue le coût du chemin ? (par séquence ? tokens totaux ? quadratique dans quelque chose ?)
• Est-ce que ça correspond ? Si le coût évolue avec les tokens totaux, le seuil devrait vérifier les tokens totaux, pas le max par séquence.

Leçon apprise : L'implémentation initiale vérifie max_ctx_seq_len (la séquence la plus longue) contre le seuil, mais le coût du chemin short-seq évolue avec les tokens packés totaux. Un batch de 100 courtes séquences pourrait déclencher incorrectement le chemin.

4. Vérifier l'État de RoPE

Quand vous ajoutez des chemins de code d'attention :

• Est-ce que apply_rotary_emb est True (l'appelant gère RoPE) ou False (rope_fusion, le backend gère RoPE) ?
• Est-ce que votre chemin applique RoPE ? Va-t-il causer une double application ?
• Avez-vous besoin de gérer les deux états de RoPE ou pouvez-vous gâter à un seul ?

5. Tracer les Limitations de la Méthode

Comprendre ce qu'une méthode NE GÈRE PAS. Quand vous réutilisez une méthode existante, tracez complètement la chaîne de dispatch au-dessus. Une méthode peut être correcte pour un scénario mais manquer les cas limites gérés par un dispatcher de haut niveau.

Exemple : forward_context_default() gère le prefill frais sans tokens KV en cache. Mais quand il y a des tokens KV en cache (contexte chunked), elle les ignore silencieusement — causant un bug de correction. Le correctif était d'appeler forward_context() à la place, qui dispatch vers :

• forward_context_with_chunked_prefill (SM100+, contexte chunked)
• forward_context_with_cached_kv (fallback SM90, ou contexte en cache)
• forward_context_default (prefill frais, pas de tokens en cache)

Liste de vérification pour réutiliser une méthode :

1. Qu'est-ce que cette méthode gère ?
2. Qu'est-ce qu'elle NE gère PAS ? (tokens en cache ? prefill chunked ? matériel spécifique ?)
3. Y a-t-il un dispatcher de haut niveau qui route vers cette méthode pour les bons cas ?
4. Devrais-je appeler le dispatcher au lieu de la méthode directement ?

6. Vérifier le Comportement Spécifique au Matériel

Le même algorithme peut avoir des propriétés numériques différentes entre les versions SM. Les kernels FMHA peuvent utiliser des implémentations internes différentes (par exemple, fusion softmax en ligne sur SM90 vs single-pass sur SM100+) qui produisent des caractéristiques de précision différentes.

Leçon apprise : Le kernel FMHA de SM90 (Hopper) avec fusion softmax en ligne pour prefill chunked a divergé de la référence single-pass d'environ 0,45 max diff — inacceptable pour un chemin critique de correction. Le correctif était de gâter le prefill chunked derrière get_sm_version() >= 100 (Blackwell+) et de revenir à forward_context_with_cached_kv sur SM90.

Quand vérifier :

• Tout nouveau chemin de code d'attention utilisant des kernels fusionnés
• Tout chemin qui change comment l'attention est divisée/chunked (prefill chunked, context parallelism)
• Quand les tolérances de précision sont strictes et le chemin traverse les générations de matériel

Flux de Travail d'Implémentation

Utiliser le Bon Niveau d'Abstraction

Choisissez les backends à partir de cette liste de priorités :

1. Méthode forward existante (par exemple, forward_context_default) — peut déjà implémenter ce dont vous avez besoin ; configurez juste l'état et dispatchez
2. Backend fusionné existant (par exemple, TrtllmAttention, FlashInferAttention) — gère les séquences packées, longueurs variables, cache KV nativement
3. Opérations fusionnées PyTorch (par exemple, F.scaled_dot_product_attention) — bon pour le prototypage mais nécessite un batching/padding manuel
4. Implémentation manuelle — dernier recours, seulement quand aucun backend existant ne convient

Utiliser le Bon Niveau d'Abstraction de Dispatch

Quand vous dispatchez vers une méthode existante, utilisez le point de dispatch le plus haut niveau qui fournit la bonne abstraction. Ne contournez pas les couches de dispatch — vous manquerez les cas limites.

Leçon apprise : L'implémentation initiale short-seq MHA appelait forward_context_default() directement. Cela fonctionnait pour le prefill frais mais supprimait silencieusement les tokens KV en cache pendant le contexte chunked. Basculer vers forward_context() (qui dispatch vers forward_context_with_cached_kv ou forward_context_with_chunked_prefill selon le cas) a corrigé le bug avec un changement d'1 ligne.

Préférer Réutiliser les Attributs Existants à Créer de Nouveaux

Quand vous ajoutez un nouveau chemin de code, vérifiez si un attribut existant peut servir double fonction :

# MAUVAIS : attribut parallèle à côté du code existant
self._short_seq_mha = create_attention(...)  # séparé de self.mha
# Puis besoin d'une gestion spéciale partout où self.mha est référencé

# BON : réutiliser l'attribut existant avec initialisation conditionnelle
if should_use_dense_mha:
    self.mha = create_attention(...)  # remplace None pour les modèles DSA
# Les chemins de code existants qui vérifient self.mha fonctionnent simplement

Leçon apprise : Le short-seq MHA utilisait initialement self._short_seq_mha comme attribut séparé pour « préserver l'assertion que self.mha is None ». Plus tard, il a été réalisé que l'assertion elle-même devrait changer (self.mqa is not None) et self.mha pouvait être réutilisé.

Exécuter Pre-Commit Avant Chaque Commit

Toujours exécuter pre-commit run --all-files avant de commiter. La branche short-seq MHA avait un commit de formatage seulement de 377 lignes (commit 15/19) qui existait uniquement parce que pre-commit n'a pas été exécuté sur les commits antérieurs. C'est de l'attention de reviewer gaspillée et pollue git blame.

# Avant chaque commit :
pre-commit run --all-files
git add -u  # ajouter au staging les fichiers auto-formatés
git commit -s -m "..."

Appliquer torch.compile Judicieusement

Extraire la Logique Partagée Immédiatement

Quand une condition apparaît en plusieurs endroits, l'extraire dans une méthode helper dans le même commit. N'attendez pas un commit de refactorisation ultérieur.

# MAUVAIS : même vérification 5-conditions en deux endroits
if (threshold > 0 and not apply_rotary and cp_size == 1 and ...):  # site 1
    ...
if (threshold > 0 and not apply_rotary and cp_size == 1 and ...):  # site 2
    ...

# BON : extraire immédiatement
def _should_use_short_mha(self, ...):
    return (threshold > 0 and not apply_rotary and cp_size == 1 and ...)

Drapeaux de Fonctionnalité pour les Optimisations Complexes

Les optimisations complexes avec plusieurs gardes, cas limites et comportement spécifique au matériel devraient être livrées désactivées par défaut. Laissez les utilisateurs s'inscrire via une variable d'environnement après les tests.

# Pattern : désactivé par défaut (threshold=0), opt-in via variable d'env
_threshold_str = os.environ.get('TRTLLM_MLA_SHORT_SEQ_MHA_THRESHOLD', '0')
self.short_seq_mha_threshold = int(_threshold_str)

Leçon apprise : L'optimisation short-seq MHA a été initialement activée par défaut (threshold=10240) au commit 8 mais a eu 18 corrections de correction supplémentaires sur les 22 commits suivants avant d'être désactivée par défaut au commit 26. Les optimisations complexes accumulent des cas limites (contexte chunked, précision SM90, sémantique du seuil) qui ne peuvent pas être découverts jusqu'à des tests larges.

Quand désactiver par défaut :

• L'optimisation a 3+ conditions de garde
• Elle touche les chemins d'attention/correction critiques
• Elle a un comportement spécifique au matériel (différentes versions SM)
• Elle n'a pas été testée dans le CI complet sur toutes les configurations

Mettre à Jour Toutes les Références Quand on Change la Sémantique

Quand vous changez ce qu'une variable/seuil signifie, grep pour TOUTES les références :

# Après changement du seuil de max_seq_len à total_packed_tokens :
grep -rn "max_seq_len\|max_ctx_seq_len\|short.*seq.*threshold" tests/ tensorrt_llm/

Mettez à jour les commentaires, docstrings, descriptions de tests et noms de variables dans le même commit.

Stratégie de Test

Quand Écrire des Tests

Leçon apprise : Les tests ont été écrits avant que le backend d'attention soit stabilisé, puis ont nécessité 5 commits séparés de fix/update alors que l'implémentation évoluait à travers 4 réécritures. Le fichier de test de 770 lignes avait besoin d'être corrigé immédiatement (bugs de placement device, layout de poids) parce qu'il n'a jamais été exécuté avant committing.

Pièges de Test Courants dans TRT-LLM

1. Les enfants non-Module ne sont pas déplacés par .to(device) : Si un module a des attributs qui ne sont pas des sous-classes nn.Module (par exemple, DSATrtllmAttention.indexer), model.to(device) ne déplacera pas leurs paramètres. Les déplacer explicitement.

2. Le layout des poids diffère de HuggingFace : Le chargement de modèle transforme les poids. Initialisez les poids de test dans le layout chargé (vérifiez modeling_*.py pour les fonctions de chargement), pas le layout du checkpoint HuggingFace.

3. Threads de fond des gestionnaires de cache : DSACacheManager et similaires créent des threads ThreadPoolExecutor qui survivent aux tests. Ajoutez pytestmark = pytest.mark.threadleak(enabled=False) au niveau du module.

4. named_parameters() manque les attributs non-Module : Quand vous copiez les poids pour les tests de comparaison A/B, copiez explicitement les paramètres des enfants non-Module (comme les poids d'indexeur).

5. Construction des métadonnées d'attention : Utilisez les fixtures/helpers de test déjà présentes dans la base de code (vérifiez tests/unittest/_torch/attention/ pour les patterns) plutôt que de construire AttentionMetadata à partir de zéro.

Consolidation de Tests

Après stabilisation de l'implémentation, prunier agressivement les tests à un ensemble minimal où chaque cas paramétralisé exerce un chemin de code distinct.

Pattern :

1. Pendant le développement, écrire des tests exhaustifs (nombreux cas paramétralisés couvrant toutes les combinaisons)
2. Après stabilisation de l'implémentation, identifier quels chemins de code chaque cas de test exerce
3. Fusionner les cas qui exercent le même chemin de code ; supprimer les cas redondants
4. Extraire les helpers de test partagés (_make_inputs, _make_metadata, _run_forward) pour réduire la duplication

Leçon apprise : Le fichier de test short-seq MHA a atteint un pic de 1394 lignes avec 21 cas paramétralisés, puis a été consolidé à 665 lignes avec 10 cas couvrant les mêmes 6 chemins de code. Trois commits séparés de nettoyage ont été nécessaires parce que la consolidation n'a pas été faite en une seule passe. Faire la consolidation comme une seule passe délibérée.

Tester sur Plusieurs Cibles Matérielles

Quand vous testez des kernels d'attention ou des opérations fusionnées, vérifiez sur plusieurs versions SM. Le même kernel peut avoir des propriétés numériques différentes sur les générations de matériel.

• SM90 (Hopper) : Fusion softmax en ligne dans FMHA — peut diverger de la référence
• SM100+ (Blackwell) : FMHA single-pass — précision numérique plus serrée
• Utilisez les gardes get_sm_version() pour sauter ou ajuster les tests par matériel

Hygiène des Commits

Pendant le Développement

Commitez librement — les commits petits et fréquents aident à suivre la progression et permettent la bissection.

Avant la Soumission de PR

Squashez les chaînes fix-on-fix en utilisant interactive rebase :

# Fusionner les commits de fix dans les commits qu'ils corrigent
git rebase -i $(git merge-base HEAD main)

Structure de commit cible pour une PR :

1. Implémentation principale — la nouvelle fonctionnalité avec toutes les gardes et cas limites
2. Optimisations supplémentaires — un commit par optimisation distincte
3. Tests — suite de test exhaustive
4. Refactorisation (optionnel) — nettoyage séparé de la fonctionnalité

Anti-patterns à Éviter

Format de Titre de PR (Conventional Commits)

Les titres de PR suivent Conventional Commits :

type: description

Types : feat, fix, perf, refactor, test, docs, chore, None

Pour les changements d'API cassants, utilisez BREAKING CHANGE: comme type pour alerter les reviewers.

Pour les développeurs NVIDIA, préfixez avec le numéro JIRA ou l'ID NVBUG :

[TRTLLM-5516] perf: description
[nvbug/5334370] fix: description

Exemples :

• feat: Add support for starcoder-v2 FP8 base + FP16/BF16 LoRA
• BREAKING CHANGE: Set default max batch size to 2048
• chore: Remove version from plugins .so
• None: Stringized enums for better error msgs
• fix https://github.com/NVIDIA/TensorRT-LLM/issues/700: a Memory leak issue in C++ runtime
• [TRTLLM-5516] perf: Replicate dummy request for cuda graph padding

Description de PR

Abordez ces points dans la description de PR :

1. Contexte/motivation : Pourquoi le changement est-il nécessaire ?
2. Résumé : Résumez les changements en un paragraphe.
3. Justification de la taille : Si la PR est volumineuse, expliquez pourquoi elle ne peut pas être divisée en plusieurs PRs.
4. Évaluation de l'impact : Impacts potentiels de performance ou fonctionnels. Signalez les risques pour les reviewers.
5. PRs associées : Liez à toute PR associée.

Concision de PR

• Évitez de commiter du code commenté.
• Chaque PR devrait traiter une seule préoccupation. S'il y a plusieurs fixes non-liés, ouvrez des PRs séparées et indiquez les dépendances dans les descriptions.

Tests de Stabilité des API

Certaines API sont protégées par la suite de test de stabilité des API. Si votre PR casse une API protégée, les tests de stabilité échoueront avec API stability validation failed. Dans ce cas, demandez l'examen des propriétaires du code API.

Impact Quantifié des Erreurs Communes

De la branche short-seq MHA (30 commits → net 2 fichiers modifiés) :

Gaspillage total : ~24 des 30 commits étaient des fixes/reverts/nettoyages du travail antérieur sur la même branche. Le changement net final est ~200 lignes dans attention.py et ~665 lignes dans les tests — réalisable en ~4-5 commits propres.

Liste de Vérification de Disponibilité pour Examen

Avant de marquer une PR prête pour examen :

• [ ] Issue GitHub créée et approuvée
• [ ] Tous les modes de parallélisation vérifiés (TP, PP, CP, EP)
• [ ] État de RoPE géré correctement (pas de double-application)
• [ ] Sémantique du seuil/garde correspond au modèle de coût
• [ ] Infrastructure existante examinée et utilisée où possible
• [ ] Logique partagée extraite (pas de conditions dupliquées)
• [ ] Commentaires/docstrings mis à jour avec tout changement sémantique
• [ ] Tests passent et couvrent les scénarios clés (incluant les tests de stabilité des API si applicable)
• [ ] Commits squashés (pas de chaînes fix-on-fix)
• [ ] Hooks pre-commit passent (pre-commit run --all-files)
• [ ] Signature DCO sur tous les commits (git commit -s)
• [ ] Les appels de dispatch utilisent le bon niveau d'abstraction (dispatcher, pas gestionnaire spécifique)
• [ ] Limitations de la méthode comprises (qu'est-ce que la méthode réutilisée NE gère PAS)
• [ ] Comportement spécifique au matériel testé (SM90, SM100+) ou gâté correctement
• [ ] Optimisations complexes désactivées par défaut avec opt-in via variable d'env
• [ ] Cas de test exercent des chemins de code distincts (pas de paramétralisations redondantes)
• [ ] Titre de PR suit le format Conventional Commits
• [ ] Description de PR aborde le contexte, le résumé et l'impact