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

Refactor metric computation in ModelOverview #1850

Merged
merged 22 commits into from
Jan 9, 2023
Merged

Refactor metric computation in ModelOverview #1850

merged 22 commits into from
Jan 9, 2023

Conversation

gaugup
Copy link
Contributor

@gaugup gaugup commented Dec 14, 2022 •
edited
Loading

Description

The idea behind the PR is to perform the metric computation once and propagate the computation into the individual ModelOverview components for use. This PR moves the metric computation as a state in ModelOverview as well.

Checklist

  • I have added screenshots above for all UI changes.
  • I have added e2e tests for all UI changes.
  • Documentation was updated if it was needed.

@gaugup
Refactor metric computation in ModelOverview
6b2b198 
Signed-off-by: Gaurav Gupta <gaugup@microsoft.com>
@github-actions
Copy link

https://responsibleai.blob.core.windows.net/pullrequest/microsoft/responsible-ai-toolbox/gaugup/RefactorModelOverForMetricComputation/dashboard/index.html

gaugup and others added 8 commits December 19, 2022 12:41
@gaugup
Move metrics to state management
666aaa7 
Signed-off-by: Gaurav Gupta <gaugup@microsoft.com>
@imatiach-msft @gaugup
add multilabel RAI vision example for fridge multilabel dataset (#1844)
9da23bb
@imatiach-msft @gaugup
fix install command for speech to text example (#1856)
7086793
@gaugup
Send the correct column name to SDK backend for model overview distri…
6d987af 
…bution charts (#1835)

Signed-off-by: Gaurav Gupta <gaugup@microsoft.com>

Signed-off-by: Gaurav Gupta <gaugup@microsoft.com>
@gaugup
Derive fairness metrics from metric stats instead of cohort stats (#1854
90371b3 
)

Signed-off-by: Gaurav Gupta <gaugup@microsoft.com>

Signed-off-by: Gaurav Gupta <gaugup@microsoft.com>
@imatiach-msft @gaugup
add upper bounds to greenlet and gevent in rai-core-flask as 5.0 rele…
87423e9 
…ase fails until they are more stable (#1852)
@imatiach-msft @gaugup
fix loading older versions of RAIInsights due to _FEATURE_METADATA ke…
db359e1 
…y error (#1857)
@gaugup
Fix build
5335d66 
Signed-off-by: Gaurav Gupta <gaugup@microsoft.com>
@github-actions
Copy link

https://responsibleai.blob.core.windows.net/pullrequest/microsoft/responsible-ai-toolbox/gaugup/RefactorModelOverForMetricComputation/dashboard/index.html

1 similar comment
@github-actions
Copy link

https://responsibleai.blob.core.windows.net/pullrequest/microsoft/responsible-ai-toolbox/gaugup/RefactorModelOverForMetricComputation/dashboard/index.html

@gaugup
Change cohort updation and deletion logic
252e9c6 
Signed-off-by: Gaurav Gupta <gaugup@microsoft.com>
@codecov-commenter
Copy link

codecov-commenter commented Dec 19, 2022 •
edited
Loading

Codecov Report

Merging #1850 (6c9a415) into main (8730515) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1850   +/-   ##
=======================================
  Coverage   93.35%   93.35%           
=======================================
  Files          93       93           
  Lines        4574     4574           
=======================================
  Hits         4270     4270           
  Misses        304      304
Flag Coverage Δ
unittests 93.35% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@github-actions
Copy link

https://responsibleai.blob.core.windows.net/pullrequest/microsoft/responsible-ai-toolbox/gaugup/RefactorModelOverForMetricComputation/dashboard/index.html

@github-actions
Copy link

https://responsibleai.blob.core.windows.net/pullrequest/microsoft/responsible-ai-toolbox/gaugup/RefactorModelOverForMetricComputation/dashboard/index.html

@github-actions
Copy link

github-actions bot commented Jan 3, 2023

https://responsibleai.blob.core.windows.net/pullrequest/microsoft/responsible-ai-toolbox/gaugup/RefactorModelOverForMetricComputation/dashboard/index.html

1 similar comment
@github-actions
Copy link

github-actions bot commented Jan 3, 2023

https://responsibleai.blob.core.windows.net/pullrequest/microsoft/responsible-ai-toolbox/gaugup/RefactorModelOverForMetricComputation/dashboard/index.html

@gaugup
Fix code review comments
24bc28a 
Signed-off-by: Gaurav Gupta <gaugup@microsoft.com>
@github-actions
Copy link

github-actions bot commented Jan 4, 2023

https://responsibleai.blob.core.windows.net/pullrequest/microsoft/responsible-ai-toolbox/gaugup/RefactorModelOverForMetricComputation/dashboard/index.html

@gaugup
Add delete logic for selected cohort index
8da6510 
Signed-off-by: Gaurav Gupta <gaugup@microsoft.com>
@github-actions
Copy link

github-actions bot commented Jan 5, 2023

https://responsibleai.blob.core.windows.net/pullrequest/microsoft/responsible-ai-toolbox/gaugup/RefactorModelOverForMetricComputation/dashboard/index.html

@gaugup
Fix code review comments part-2
a2fc858 
Signed-off-by: Gaurav Gupta <gaugup@microsoft.com>
@gaugup
Remove unnecessary condition
7466199 
Signed-off-by: Gaurav Gupta <gaugup@microsoft.com>
@romanlutz
Copy link
Contributor

This is breaking the cohort default selection in the chart configuration:

Your PR:
image

Previously:
image

I would also recommend making sure that it's robust to cohort changes (adding new cohort, deleting a cohort, etc.).

@github-actions
Copy link

github-actions bot commented Jan 5, 2023

https://responsibleai.blob.core.windows.net/pullrequest/microsoft/responsible-ai-toolbox/gaugup/RefactorModelOverForMetricComputation/dashboard/index.html

1 similar comment
@github-actions
Copy link

github-actions bot commented Jan 5, 2023

https://responsibleai.blob.core.windows.net/pullrequest/microsoft/responsible-ai-toolbox/gaugup/RefactorModelOverForMetricComputation/dashboard/index.html

@github-actions
Copy link

github-actions bot commented Jan 5, 2023

https://responsibleai.blob.core.windows.net/pullrequest/microsoft/responsible-ai-toolbox/gaugup/RefactorModelOverForMetricComputation/dashboard/index.html

@gaugup
Copy link
Contributor Author

gaugup commented Jan 5, 2023 •
edited
Loading

This is breaking the cohort default selection in the chart configuration:

Your PR: image

Previously: image

I would also recommend making sure that it's robust to cohort changes (adding new cohort, deleting a cohort, etc.).

The link https://responsibleai.blob.core.windows.net/pullrequest/microsoft/responsible-ai-toolbox/gaugup/RefactorModelOverForMetricComputation/dashboard/index.html doesn't work for me. I don't see the issue locally.

image

I don't change any logic around cohort selection as such. Not sure why you would see this issue.

@romanlutz
Copy link
Contributor

romanlutz commented Jan 5, 2023 •
edited
Loading

I don't change any logic around cohort selection as such. Not sure why you would see this issue.

The selectedDatasetCohorts should be exactly what goes into that panel, so while you didn't touch the panel you are touching the data that goes into it.

That said, after checking out your branch locally this particular issue isn't reproducing for me either. We should really have better test coverage so that we don't have to worry as much about these things breaking 😢

@github-actions
Copy link

github-actions bot commented Jan 9, 2023

https://responsibleai.blob.core.windows.net/pullrequest/microsoft/responsible-ai-toolbox/gaugup/RefactorModelOverForMetricComputation/dashboard/index.html

@github-actions
Copy link

github-actions bot commented Jan 9, 2023

https://responsibleai.blob.core.windows.net/pullrequest/microsoft/responsible-ai-toolbox/gaugup/RefactorModelOverForMetricComputation/dashboard/index.html

@github-actions
Copy link

github-actions bot commented Jan 9, 2023

https://responsibleai.blob.core.windows.net/pullrequest/microsoft/responsible-ai-toolbox/gaugup/RefactorModelOverForMetricComputation/dashboard/index.html

@gaugup gaugup enabled auto-merge (squash) January 9, 2023 21:47
@gaugup gaugup merged commit 980b893 into main Jan 9, 2023
@gaugup gaugup deleted the gaugup/RefactorModelOverForMetricComputation branch January 9, 2023 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@romanlutz romanlutz romanlutz approved these changes

@xuke444 xuke444 Awaiting requested review from xuke444 xuke444 is a code owner

@tongyu-microsoft tongyu-microsoft Awaiting requested review from tongyu-microsoft

@vinuthakaranth vinuthakaranth Awaiting requested review from vinuthakaranth vinuthakaranth is a code owner

@RubyZ10 RubyZ10 Awaiting requested review from RubyZ10 RubyZ10 is a code owner

@hawestra hawestra Awaiting requested review from hawestra

@imatiach-msft imatiach-msft Awaiting requested review from imatiach-msft

@kicha0 kicha0 Awaiting requested review from kicha0

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@gaugup @codecov-commenter @romanlutz @imatiach-msft