Open RobertBaruch opened 1 year ago
The comment on Trainer.push_to_hub
does say Upload *self.model* and *self.tokenizer* to the π€ model hub
. And in fact, it does call the trainer's tokenizer.save_pretrained
function. However, in run_speech_recognition_ctc.py
, tokenizer
is set to feature_extractor
in the initialization, and Wav2Vec2FeatureExtractor.save_pretrained
does not save tokenizer settings.
When I replace these lines at the end of run_speech_recognition_ctc
from this:
if training_args.push_to_hub:
trainer.push_to_hub(**kwargs)
else:
trainer.create_model_card(**kwargs)
to this:
tokenizer.save_pretrained(training_args.output_dir)
trainer.create_model_card(**kwargs)
if training_args.push_to_hub:
trainer.push_to_hub(**kwargs)
we do get tokenizer files. Also, may as well write the model card in any case.
cc @sanchit-gandhi
The code in the run_speech_recognition_ctc.py
script as well as the instructions from the ASR guide that you used in issue https://github.com/huggingface/transformers/issues/23188 do the following:
trainer = Trainer(
...
tokenizer=processor.feature_extractor,
...
)
The "processor" combines the feature extractor and tokenizer into a single class, but because we only pass the feature extractor to the Trainer, the tokenizer doesn't get saved. So that's clearly a mistake on our end.
The following fix should work:
trainer = Trainer(
...
tokenizer=processor,
...
)
We're updating the docs to fix this. (It's a bit confusing that this argument from Trainer is called tokenizer
but that's what's responsible for saving the non-model stuff.)
Probably we can directly add a new argument to the Trainer
for the processor @hollance? This would stop all confusion IMO:
trainer = Trainer(
...
processor=processor,
...
)
Here we could expect the user to pass either one of tokenizer
or processor
to the Trainer
. Within the Trainer
we only use the tokenizer
to get the model input name, which after #20117 we can now get directly from the processor
.
Can confirm, setting tokenizer=processor
in run_speech_recognition_ctc.py
works. Agree that tokenizer
is a bit of a misleading keyword then.
Keeping this open since we really should update the Trainer to take processor
as an argument over tokenizer=processor
This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.
Please note that issues that do not follow the contributing guidelines are likely to be ignored.
System Info
transformers
version: 4.28.1Who can help?
@sgugger
Information
Tasks
examples
folder (such as GLUE/SQuAD, ...)Reproduction
Run training using run_speech_recognition_ctc.py and the included json file.
train.json.zip
Next, attempt to infer using the trained model:
Output:
Checking the uploaded repo, it seems that no tokenizer-related files (e.g.
vocab.json
,tokenizer_config.json
, etc) were pushed.I added some debug to
run_speech_recognition_ctc.py
and found that these files were generated locally, but got deleted locally during step 7 whenTrainer
was initialized (line 701).The output from
run_speech_recognition_ctc.py
at that point was:It seems that instantiating
Training
withpush_to_hub=true
creates a new repo and then empties anything in the local directory so that it can clone the repo into it. This deletes any files written to the local directory, which includes the tokenizer configs.Expected behavior
No error.