KEMBAR78
Add PR curves and F1 score to WIT by jameswex · Pull Request #2264 · tensorflow/tensorboard · GitHub
Skip to content

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
<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