chore(linter): add TRF018 modeling rule#46259
Conversation
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
tarekziade
changed the title
|
[For maintainers] Suggested jobs to run (before merge) run-slow: albert, align, altclip, autoformer, bit, blt, bridgetower, chinese_clip, clap, clip, clipseg, clvp, colmodernvbert, colpali, colqwen2, conditional_detr |
|
run-slow: albert, align, altclip, autoformer, bit, blt, bridgetower, chinese_clip, clap, clip, clipseg, clvp, colmodernvbert, colpali, colqwen2, conditional_detr |
|
This comment contains models: ["models/albert", "models/align", "models/altclip", "models/autoformer", "models/bit", "models/blt", "models/bridgetower", "models/chinese_clip", "models/clap", "models/clip", "models/clipseg", "models/clvp", "models/colmodernvbert", "models/colpali", "models/colqwen2", "models/conditional_detr"] |
|
View the CircleCI Test Summary for this PR: https://huggingface.co/spaces/transformers-community/circle-ci-viz?pr=46259&sha=ddd8a0 |
vasqu
left a comment
vasqu
left a comment
There was a problem hiding this comment.
Please see my first comment. Imo, we are not actually utilizing what the goal behind this is: removing unnecessary code that does the same as the parent
This cannot really be caught with an ast rule imo but we should use the opportunity to refactor the affected models here
| def _init_weights(self, module): | ||
| + super()._init_weights(module) | ||
| if isinstance(module, AcmeCustomLayer): | ||
| module.gate.data.zero_() |
There was a problem hiding this comment.
| module.gate.data.zero_() | |
| init.zeros(module.gate) |
we are using the init module to initialize just as nit
| @torch.no_grad() | ||
| def _init_weights(self, module): | ||
| """Initialize the weights.""" | ||
| super()._init_weights(module) |
There was a problem hiding this comment.
Imo, if we add this, we also should check the code below whether we can remove parts. For example right below, the linear init looks very standard and likely could come from the parent instead
There was a problem hiding this comment.
I don't thin this can be done with a static rule so we have to be really picky during review to catch these + here now
| description = "`trust_remote_code` should never be used in native model integrations." | ||
| default_enabled = true | ||
| allowlist_models = [] | ||
| allowlist_models = ["auto"] |
There was a problem hiding this comment.
same here as in the other PR, why is/was this actually needed 👀
3 participants
What does this PR do?
Adds support for mlinter TRF018