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

[AiLab] Use new fields in Model Cards #40166

Merged
merged 6 commits into from
Apr 23, 2021
Merged

[AiLab] Use new fields in Model Cards #40166

merged 6 commits into from
Apr 23, 2021

Conversation

Erin007
Copy link
Contributor

@Erin007 Erin007 commented Apr 20, 2021

Continuation of the work in #40100. Rather than metadata.labelColumn we now use metadata.label and instead of metadata.selectedFeatures we pull info from metadata.features for the ModelCard component.

If the model is too old to have these new data fields, the model card doesn't render:
Screen Shot 2021-04-20 at 12 03 46 PM

For models that do have the new fields, we now show more details about the label and features in the Model Card, including possible values and max/min where appropriate:
Screen Shot 2021-04-20 at 11 36 14 AM

The Curriculum team has been notified of this change; they have re-built all of the necessary models in the curriculum pilot to include the new fields.

Possible Values:{' '}
{!feature.values && (
<p style={styles.details}>
min: {feature.min} max: {feature.max}
Copy link
Member

Choose a reason for hiding this comment

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

Is this our preferred presentation for ranges across the whole experience? Or should we try and figure out a unified design?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are three places where we expose max and min to the user:

Test model in AI Lab
Screen Shot 2021-04-22 at 9 28 14 AM

App Lab auto gen design element
Screen Shot 2021-04-22 at 9 30 02 AM

App Lab model card (this PR)
Screen Shot 2021-04-22 at 9 17 07 AM

The only difference is whether we use ( ) or not, which seems to me to make sense in the above contexts. If you have ideas for an improvement on how to display this information, we can consider it, but in terms of consistency I think the change I've made in this PR for model cards matches the others - single line, min first, then max with colons.

Copy link
Member

Choose a reason for hiding this comment

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

Seems fair. My only thought is that we could put a comma between them in the third variant...

@Erin007 Erin007 merged commit 4b43b5e into staging Apr 23, 2021
@Erin007 Erin007 deleted the new-field-model-cards branch April 23, 2021 17:19
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.

None yet

2 participants