Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Improvement] speed up confusion matrix calculation #465

Merged
merged 2 commits into from
Dec 21, 2020
Merged

[Improvement] speed up confusion matrix calculation #465

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
626e073
a little bit faster confusion matrix
dreamerlin Dec 19, 2020
1d773d4
add changelog
dreamerlin Dec 20, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/changelog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
- Support training and testing for Spatio-Temporal Action Detection ([#351](https://github.com/open-mmlab/mmaction2/pull/351))
- Fix CI due to pip upgrade ([#454](https://github.com/open-mmlab/mmaction2/pull/454))
- Add markdown lint in pre-commit hook ([#255](https://github.com/open-mmlab/mmaction2/pull/225))
- Speed up confusion matrix calculation ([#465](https://github.com/open-mmlab/mmaction2/pull/465))
- Use title case in modelzoo statistics ([#456](https://github.com/open-mmlab/mmaction2/pull/456))
- Add FAQ documents for easy troubleshooting. ([#413](https://github.com/open-mmlab/mmaction2/pull/413), [#420](https://github.com/open-mmlab/mmaction2/pull/420), [#439](https://github.com/open-mmlab/mmaction2/pull/439))

Expand Down
17 changes: 11 additions & 6 deletions mmaction/core/evaluation/accuracy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,17 @@ def confusion_matrix(y_pred, y_real, normalize=None):

label_set = np.unique(np.concatenate((y_pred, y_real)))
num_labels = len(label_set)
label_map = {label: i for i, label in enumerate(label_set)}
confusion_mat = np.zeros((num_labels, num_labels), dtype=np.int64)
for rlabel, plabel in zip(y_real, y_pred):
index_real = label_map[rlabel]
index_pred = label_map[plabel]
confusion_mat[index_real][index_pred] += 1
max_label = label_set[-1]
label_map = np.zeros(max_label + 1, dtype=np.int64)
Copy link
Member

Choose a reason for hiding this comment

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

I think it's OK to use a dictionary instead of np array here, as the original one:

label_map = {label: i for i, label in enumerate(label_set)}

so that 4 lines -> 1 line
any performance concern here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

label_map = {label: i for i, label in enumerate(label_set)}
y_pred_mapped = [label_map[i] for i in y_pred]
y_real_mapped = [label_map[i] for i in y_real]

This is much slower even than the original one.

Copy link
Contributor

Choose a reason for hiding this comment

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

numbers would be appreciated

Copy link
Member

Choose a reason for hiding this comment

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

That's right, the for loop is much slower.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The proposal by kenny VS The original one before this PR:
for classes = 10, length = 100, times = 10000, 1.2643 vs 1.1462
for classes = 10, length = 100, times = 10000, 0.6333 vs 0.5554
even worse

for i, label in enumerate(label_set):
label_map[label] = i

y_pred_mapped = label_map[y_pred]
y_real_mapped = label_map[y_real]

confusion_mat = np.bincount(
num_labels * y_real_mapped + y_pred_mapped,
minlength=num_labels**2).reshape(num_labels, num_labels)

with np.errstate(all='ignore'):
if normalize == 'true':
Expand Down