Skip to content

Conversation

@spencerthomas1722
Copy link
Collaborator

No description provided.

if trainer.is_world_process_zero():
if training_args.do_train:
trainer.save_model()
trainer.save_model() # NOTE: a RobertaConfig is loaded here. why?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did you want to keep this in here?

raise NotImplementedError(
"This functionality has not been restored yet"
)
model = CnlpModelForClassification(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this definition will load a fine-tuned cnlpt model as an encoder, but re-initialize the classifier head. This would be the expected behavior for some use cases, but missing some use cases. I think we want to edit this to explicitly handle the two different cases (even if one still throws an exception), rather than having the user guess what might be happening. We should force them to specify whether to keep or ignore existing classifiers (as in the hier model).

return compute_metrics_fn

# Initialize our Trainer
training_args.load_best_model_at_end = True
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tmills I removed this line because it makes it impossible to do prediction without training; it looks for a model checkpoint in the output dir, but when do_train = False, that doesn't exist.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants