nnsight icon indicating copy to clipboard operation
nnsight copied to clipboard

UnifiedTransformer -> nnsight0.5

Open AdamBelfki3 opened this issue 9 months ago • 3 comments

Reintroduce the implementation of the UnifiedTransformer class to work with the latest version nnsight. Add unit tests to validate this functionality.

AdamBelfki3 avatar Mar 13 '25 18:03 AdamBelfki3

Any updates here? We're using UnifiedTransformer in one of our baseline implementations for MIB, and it would be super helpful to have in a more recent version!

aaronmueller avatar May 17 '25 12:05 aaronmueller

There's a discussion Jaden wants to have about whether an 0.5 feature (operation hooking) makes it unneeded to have UnifiedTransformer in this version.

davidbau avatar May 19 '25 21:05 davidbau

Yeah I think it might makes more sense to implement a new class using renaming e.g. like this and predefine hook functions per model to grab attention etc rather than using TL as a backend.

Pro:

  • Official impl -> no precision issue

Cons:

  • If HF change the name of intermediate variable this would break while TL impl would remain more stable

Butanium avatar Jun 20 '25 16:06 Butanium