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.

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

Add PR curves and F1 score to WIT #2264

Merged
merged 3 commits into from
May 23, 2019
Merged

Add PR curves and F1 score to WIT #2264

merged 3 commits into from
May 23, 2019

Conversation

jameswex
Copy link
Contributor

  • Motivation for features / changes

Per user requests, adding PR curves and F1 score to performance tab of WIT for binary classification models.

  • Technical description of changes
  • Adjust chart generation code to handle both PR curves and existing ROC curves
  • Change visuals to handle both charts side by side
  • Add calculation of precision (positive predictive value) in order to support PR curve.
  • Add calculation of F1 score
  • Add row in performance table for F1 score
  • Screenshots of UI changes

pr

  • Detailed steps to verify changes work correctly (as executed by you)

Ran binary classification demos (single and multimodel) to visualize change and ensure correctness.

@jameswex
Copy link
Contributor Author

@tolga-b please review

Copy link
Contributor

@tolga-b tolga-b left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@jameswex jameswex requested a review from stephanwlee May 23, 2019 18:04
@@ -1808,6 +1840,11 @@ <h2>Show similarity to selected datapoint</h2>
<div class="perf-table-num-entry">[[getAccuracyModelIndex(inferenceStats_, featureValueThreshold.threshold, index, featureValueThreshold)]]</div>
</template>
</div>
<div class="perf-table-f1 perf-table-clickable" on-tap="togglePerfRow">
Copy link
Contributor

Choose a reason for hiding this comment

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

No action required (since we really don't honor this): a div that is clickable is not really accessible. At the very least, we should put tab-index="-1" (and of course role=button)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ack

if (Object.keys(confCounts).length == 0) {
return 0;
}
if (confCounts['Yes']['Yes'] == 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

might be more readable (or more maintainable) to assign this to a variable.

const truePositive = confCoujnts['Yes']['Yes'];
...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@jameswex jameswex merged commit 6826df7 into tensorflow:master May 23, 2019
@jameswex jameswex deleted the f1 branch May 23, 2019 20:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants