Add AST (Audio Spectrogram Transformer) Adapter - #1484
Conversation
|
Thanks for putting this together @dylanberens! The AST domain work is great, the Q/K/V/O and bias reshapes, MLP transposes, the overlapping-patch Conv2d, the cls-distillation-patches token order, and the pooled dual-token + dual-LayerNorm head are all correct. We will need to address & adjust the approach: this is written against the legacy HookedTransformer Primary issues with the current code:
To make this mergeable:
Also, a couple repo items: make sure to drop Happy to re-review once it's on the TransformerBridge pattern. |
bc7afb1 to
3b9db10
Compare
|
thanks @jlarson4 for the feedback and roadmap! I've refactored the AST adapter to follow the V3 TransformerBridge pattern against dev
ready for re-review when you get a chance, thanks jonah! |
|
Excellent, thank you @dylanberens! I will make sure to let you know when I get around to reviewing |
jlarson4
left a comment
There was a problem hiding this comment.
Hey @dylanberens sorry it took me so long to get around to reviewing this. Full detailed review broken down below. The overall structure and content of your adapter are very good! There are just a couple small items that need addressing before I can merge. Once the comments below are resolved, this should be good to go!
re add ast import and add to factory :wq y wq :wq
3a2a164 to
a942acc
Compare
|
Thanks @dylanberens for the update! The fixes look good, I verified them all locally. Three things left before merge:
|
Description
Refactored the AST architecture adapter to align with the V3 'TransformerBridge' architecture and target 'dev'.
Key Changes
Fixes # 1484
Type of change
Checklist: