-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Confusion matrix metric #1921
base: main
Are you sure you want to change the base?
Confusion matrix metric #1921
Conversation
This looks excellent! Which task(s) were you thinking of adding it to? "All multiple choice tasks with two options," or something else? |
@StellaAthena Thank you! Multiple options are supported too. For example
Although it might make sense to put out a disclaimer since most multiple choice tasks are not necessarily classification tasks (for example most QA type tasks). But it still might be useful if people wanted to check for A-bias (the tendency of a model towards picking the answer choice labeled “A"). |
761345e
to
8a15ff5
Compare
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.
Hi @minaremeli , thanks for the PR! This looks like a very cool contribution.
I'm however hesitant to have this printed every run. It also might be nice to be able to apply this to acc_norm
as well as acc
.
Would you be willing to make this a scripts/confusion_matrix.py
utility that can take in the path to a logged per-sample results file as well as a target (multiple-choice) metric, and write the confusion matrix for that task? That way we can promote it as something for a user to use when exploring their results, but not end up printing too much info every time.
@haileyschoelkopf Thank you for your for your feedback! I can see how the current solution might crowd the stdout, especially if multiple tasks are being evaluated... I would be happy to change the PR to fit your suggestions. I like your idea of moving it to the per-sample results file (or even a separate file?). Do you suggest to simply append the CM to that file? And should CM stay a metric to be included in the config file of the task or would it make more sense as a command line argument like |
I was suggesting that we have a workflow like
How does this sound? |
@minaremeli hey, I wanted to follow up about this and see what your progress is / if you're stuck on anything. |
Hey @StellaAthena! No progress yet, I haven't had time to start working on this change up until now. Let me get back to you on Friday |
Modified implementation as per the proposal in EleutherAI#1921.
This reverts commit 8a15ff5.
results = sample["resps"] | ||
lls = [float(r[0][0]) for r in results] | ||
pred = np.argmax(lls) | ||
pred_norm = np.argmax(lls / completion_len) | ||
if metric == "acc": | ||
pred_label = choices[pred] | ||
elif metric == "acc_norm": | ||
pred_label = choices[pred_norm] | ||
predictions.append(pred_label) |
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.
Prediction method (acc
or acc_norm
) copied based on this. A drawback of this is that if the original method changes (e.g. normalization constant changes from number of characters to number of tokens), this will not be reflected here.
if isinstance(target, int): | ||
target_label = choices[target] | ||
targets.append(target_label) | ||
elif isinstance(target, str): | ||
assert target in choices | ||
targets.append(target) | ||
elif isinstance(target, list): | ||
raise NotImplementedError( | ||
"No support yet for multi-label confusion matrix!" | ||
) |
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.
Target can be specified asint
(index of true label) or str
(true label) or list
(multi-label tasks). I decided to not implement the multi-label case.
@haileyschoelkopf @StellaAthena I changed my implementation to the proposed script version. I also left some comments on parts that might require your input. What's your suggestion regarding documentation? |
Also, I noticed that the sample results files have changed recently from |
For classification tasks it is often useful to plot the confusion matrix.
Implementation details
Added a new metric called
cm
tometrics.py
, with aggregation function that calls sklearn'sconfusion_matrix()
.In
utils.py
, added a new functionmake_confusion_matrix()
which is called aftermake_table()
in___main___.py
.Example
For example, by adding
cm
to the list of evaluated metrics for thesst2
task:lm_eval/tasks/glue/sst2/default.yaml
And executing the following command:
We get this output:
Where the rows represent the true classes (
C0
andC1
, which correspond to thenegative
andpositive
classes) and the columns represent the predicted classes.