Skip to content

Resolve OOM issue + better examples for classification#847

Merged
czaloom merged 5 commits into
mainfrom
czaloom-clf-conf-mat-oom-844
Jun 2, 2025
Merged

Resolve OOM issue + better examples for classification#847
czaloom merged 5 commits into
mainfrom
czaloom-clf-conf-mat-oom-844

Conversation

@czaloom

@czaloom czaloom commented May 30, 2025

Copy link
Copy Markdown
Collaborator

Changes

  • Resolves OOM issue
    • Reduced Confusion Matrix memory complexity from (n_score_thresholds * n_labels * n_datums * n_examples) to (n_score_thresholds * n_labels * n_datums)
  • Remove number_of_examples option from classification
  • Renamed Exceptions to Errors

@czaloom czaloom self-assigned this May 30, 2025
@czaloom czaloom linked an issue May 30, 2025 that may be closed by this pull request
1 task
@czaloom czaloom changed the title better examples for classification Resolve OOM issue + better examples for classification May 30, 2025
@czaloom czaloom marked this pull request as ready for review May 30, 2025 21:09
if idx < n_matched:
glabel = index_to_label[unique_matches[idx, 1]]
plabel = index_to_label[unique_matches[idx, 2]]
confusion_matrix[glabel][plabel]["count"] += 1

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

now that we're giving all examples is it always true that the count here is just the length of the examples entry? I worry about redundant stuff like this because if these get out of sync it's confusing... Can we just return the examples list and let callers take the len of that? Or is that a breaking interface change?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I will let Charles answer but i think we are going to try to separate "here is the matrix" from "here are the examples" to allow the UI to be more svelt showing the first, and more precise on the second

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It is a bit redundant now but the intent is to eventually split the examples from the counts entirely. We are still implementing a number of examples cap on the chariot-valor side.

@czaloom czaloom merged commit 15f08d0 into main Jun 2, 2025
4 checks passed
@czaloom czaloom deleted the czaloom-clf-conf-mat-oom-844 branch June 2, 2025 14:32
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.

BUG: Classification confusion matrix OOM

3 participants