-
Notifications
You must be signed in to change notification settings - Fork 911
PEFT on Named Entity Recognition #1336
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
08d15ca
to
0fdd48b
Compare
stanza/models/ner/trainer.py
Outdated
|
||
|
||
# IMPORTANT: gradient checkpointing BREAKS peft if applied before | ||
# 1. Apply PEFT FIRST (looksie! its above this line) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's
stanza/models/ner/trainer.py
Outdated
@@ -85,7 +111,10 @@ def __init__(self, args=None, vocab=None, pretrain=None, model_file=None, device | |||
if pname.split('.')[0] not in exclude: | |||
p.requires_grad = False | |||
self.model = self.model.to(device) | |||
self.optimizer = utils.get_optimizer(self.args['optim'], self.model, self.args['lr'], momentum=self.args['momentum'], bert_learning_rate=self.args.get('bert_learning_rate', 0.0)) | |||
if not second_optim: | |||
self.optimizer = utils.get_optimizer(self.args['optim'], self.model, self.args['lr'], momentum=self.args['momentum'], bert_learning_rate=self.args.get('bert_learning_rate', 0.0), is_peft=bool(self.args.get("peft"))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these are still using the old peft
argument it appears
stanza/models/ner/trainer.py
Outdated
@@ -15,6 +15,9 @@ | |||
from stanza.models.ner.vocab import MultiVocab | |||
from stanza.models.common.crf import viterbi_decode | |||
|
|||
from peft import get_peft_model_state_dict, set_peft_model_state_dict |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh! one last thing. unless / until we've decided to make these required, we should hide these imports in the functions that use them. i can make that change
…t a requirement yet
… note on something we need to do in the bert test
depends on #1337 and #1338 (merged)