Conversation
|
@vasqu I did also change the |
vasqu
left a comment
vasqu
left a comment
There was a problem hiding this comment.
Looks overall good. Fixing the init should be a priority so that the sequence classification model can be imported from transformers.
|
Last thing but can we also add the new sequence model to |
|
@vasqu Thank you so much for the detailed PR review! I resolved most comments, could you take a look? |
|
@Adibvafa Will take a closer relook in a bit and good work! |
vasqu
left a comment
vasqu
left a comment
There was a problem hiding this comment.
Only the stuff I commented on otherwise LGTM!
There was a problem hiding this comment.
Tests seem to fail and I'm not sure why. Could you look into it? Not all tests should pass, but especially the ones already written (i.e. the ones that also pass in main).
Let's revert some stuff and really focus on only adding the sequence classifier first and handle renames etc separately.
Some fixes for the stuff introduced in PR #1
2 participants
No description provided.