-
Notifications
You must be signed in to change notification settings - Fork 31.9k
Fix doc example #16448
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
Fix doc example #16448
Conversation
|
The documentation is not available anymore as the PR was closed or merged. |
| >>> # To train a model on `num_labels` classes, you can pass `num_labels=num_labels` to `.from_pretrained(...)` | ||
| >>> num_labels = len(model.config.id2label) | ||
| >>> model = {model_class}.from_pretrained("{checkpoint}", num_labels=num_labels) | ||
| >>> model = {model_class}.from_pretrained( |
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.
This I think we should revert. Why wouldn't this work?
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.
I had a previous conversation with @NielsRogge on Slack.
He was using celine98/canine-s-finetuned-sst2, which has "problem_type": "single_label_classification", set in the config.
Due to setting, in the following block,
transformers/src/transformers/models/canine/modeling_canine.py
Lines 1322 to 1329 in d55fcbc
| if labels is not None: | |
| if self.config.problem_type is None: | |
| if self.num_labels == 1: | |
| self.config.problem_type = "regression" | |
| elif self.num_labels > 1 and (labels.dtype == torch.long or labels.dtype == torch.int): | |
| self.config.problem_type = "single_label_classification" | |
| else: | |
| self.config.problem_type = "multi_label_classification" |
the block for the condition self.config.problem_type is None is not run, and it continue to be single_label_classification, therefore the output is not compatible with the provided labels (which is to work with multiple labels here).
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.
For Roberta, cardiffnlp/twitter-roberta-base-emotion is used, and the problem_type is not set in the config. Therefore, the model code is able to set it to multi_label_classification.
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.
Happy with "force-passing" single_label_classification as a flag here to overwrite default configs
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.
You mean multi_label_classification? It's ok to keep this change?
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.
I think it is in the reversed direction:
-
celine98/canine-s-finetuned-sst2has config is set tosingle_label_classification) -
but this block in
PT_SEQUENCE_CLASSIFICATION_SAMPLE
transformers/src/transformers/utils/doc.py
Lines 269 to 282 in d77680e
```python >>> # To train a model on `num_labels` classes, you can pass `num_labels=num_labels` to `.from_pretrained(...)` >>> num_labels = len(model.config.id2label) >>> model = {model_class}.from_pretrained( ... "{checkpoint}", num_labels=num_labels, problem_type="multi_label_classification" ... ) >>> labels = torch.nn.functional.one_hot(torch.tensor([predicted_class_id]), num_classes=num_labels).to( ... torch.float ... ) >>> loss = model(**inputs, labels=labels).loss >>> loss.backward() # doctest: +IGNORE_RESULT ``` """ is meant to be
multi_label_classification. Seelabels = torch.nn.functional.one_hot. -
That's why @NielsRogge needs to add
problem_type="multi_label_classification"in the call tofrom_pretrained.
I will wait @NielsRogge joining this discussion, since he knows better the reason behind his change.
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.
Well, he is faster than my response ...
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.
Ah I see sorry yes you're right - ok to keep the change for me then!
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.
Nono all good this makes perfect sense, I misunderstood here.
What does this PR do?
This PR fixes the doc example of
xxxForSequenceClassificationmodels. I wonder how this test passes currently, cause for me it returned an error as the labels are of shape(batch_size, num_labels)but theproblem_typewasn't set to "multi_label_classification".