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

[BUG] Bias charts auto-refresh with stale data #1403

Closed
alexcreasy opened this issue Jun 16, 2023 · 1 comment · Fixed by #1404
Closed

[BUG] Bias charts auto-refresh with stale data #1403

alexcreasy opened this issue Jun 16, 2023 · 1 comment · Fixed by #1404
Assignees
@alexcreasy
Labels
feature/explainability feature/model-serving Model Serving Feature kind/bug Something isn't working priority/high Important issue that needs to be resolved asap. Releases should not have too many of these.
Milestone
Explainability Ph...

Comments

@alexcreasy
Copy link
Contributor

alexcreasy commented Jun 16, 2023 •
edited
Loading

The bias metric charts do not auto refresh correctly. Whilst the dashboard is pulling data from prometheus on the required cadence, each refresh returns the same data as the one before. The effect of this can be seen below.

Demonstration

Data was regularly pushed to prometheus and the charts left to auto update on a 30 second cadence.

  • Notice on the first image that after a few minutes of receiving data and several auto refreshes, no new data has been plotted (evidenced by the gap)
  • The second image shows the same charts immediately after a page refresh, all the data that was missed previously is visible

localhost_4010_modelServing_metrics_trustyai-e2e-modelmesh_demo-loan-nn-onnx-alpha_bias (7)_annotated

localhost_4010_modelServing_metrics_trustyai-e2e-modelmesh_demo-loan-nn-onnx-alpha_bias (8)

Cause

The useModelServingMetrics hook[1] sets up the queries to poll using the end value (initialised as Date.now()). This initial value will be used on every subsequent refresh (unless calling the refresh method of the context, this will update the end value and cause all data streams to fetch the latest data).

This is because the useQueryRangeResourceData hook[2] wraps the usePrometheusQueryRange hook[3] in a useContextResourceData hook. This hook simply calls the FetchState refresh method on an interval, however that means the query made at [3] will always have the same start and end value set in the initial call, until a manual refresh is made.

[1]

odh-dashboard/frontend/src/api/prometheus/serving.ts

Line 34 in 9f369a6

const [end, setEnd] = React.useState(lastUpdateTime);

[2]

odh-dashboard/frontend/src/api/prometheus/useQueryRangeResourceData.ts

Line 56 in 9f369a6

return useContextResourceData<TrustyData>(

[3]

odh-dashboard/frontend/src/api/prometheus/usePrometheusQueryRange.ts

Line 25 in 9f369a6

const endInS = endInMs / 1000;

@alexcreasy alexcreasy added kind/bug Something isn't working priority/high Important issue that needs to be resolved asap. Releases should not have too many of these. feature/model-serving Model Serving Feature feature/explainability labels Jun 16, 2023
@alexcreasy alexcreasy added this to the Explainability Phase 1 milestone Jun 16, 2023
@github-project-automation github-project-automation bot moved this to Needs prioritization in ODH Dashboard Planning Jun 16, 2023
@alexcreasy alexcreasy mentioned this issue Jun 16, 2023
Closed
@alexcreasy alexcreasy self-assigned this Jun 16, 2023
@alexcreasy alexcreasy moved this from Needs prioritization to In progress in ODH Dashboard Planning Jun 16, 2023
alexcreasy added a commit to alexcreasy/odh-dashboard that referenced this issue Jun 16, 2023
@alexcreasy
Fixes: Bias charts auto-refresh with stale data (opendatahub-io#1403)
6acd315
@alexcreasy alexcreasy mentioned this issue Jun 16, 2023
Merged
6 tasks
@alexcreasy
Copy link
Contributor Author

Pull request raised needs review:

@alexcreasy alexcreasy linked a pull request Jun 20, 2023 that will close this issue
Fixes issue with bias charts auto-refreshing with stale data (#1403) #1404
Merged
6 tasks
openshift-merge-robot pushed a commit that referenced this issue Jun 20, 2023
@alexcreasy
Fixes issue with bias charts auto-refreshing with stale data (#1403) (#…
64a8012 
…1404)

* Refactor prometheus queries to remove duplication

* Fix graph not refreshing issue

* Add code review suggestions
@github-project-automation github-project-automation bot moved this from In progress to Done in ODH Dashboard Planning Jun 20, 2023
openshift-merge-robot pushed a commit that referenced this issue Jun 30, 2023
@DaoDaoNoCode @andrewballantyne @alexcreasy
Bias to incubation (#1449)
bb8e2c5 
* Re-enable Metrics

* Fix Graphs & Configure Infrastructure (#1023)

* Rework Inference Metrics & Add Runtime Metrics (invalid quieries)

* Add stacked line chart functionality

* Trustyai demo phase0 (#1093)

* Explainability: Fairness and Bias Metrics (Phase 0) (#1001) (#1006) (#1007) (#1008)
  - Initial feature set for TrustyAI related UI functionality
  - Adds tab based navigation to modelServing screen
  - Adds a bias metrics tab with charts for visualising SPD and DIR metrics
  - Enhances prometheus query features for accessing TrustyAI data
  - Enhacements to MetricsChart component making it more configurable

* Update key of request name to match trusty backend

* Remove unnecessary div and inline style from tooltip

* Remove 15 minutes refresh option

* Prefer optional prop to type union with undefined

* Move function definitions inline

* Prefer narrowing over type conversion

* Inline tab change handler

* Remove toolbar option from ApplicationsPage

* Inline domain calculator functions

* Move defaultDomainCalculator to utils

* Return null instead of undefined

* Use threshold label instead of index for key

* Add enum for tab keys

* Remove magic numbers from domain calculations

* Make ResponsePredicate mandatory and add predicate to useQueryRangeResourceData

* TrustyAI Client (#1318)

* Add support for insecure http requests in development mode

* Adds low level API client for TrustyAI service

* Adds TrustyAI high level API and contexts

* Get scheme of TrustyAI route from k8s data

* Add model bias configuration table (#1290)

* Add model bias configuration table

* rebase and remove mock data

* Update Trusty AI client to handle API changes (#1336) (#1337)

* Add bias metrics configuration modal (#1343)

* Add configuration modal

* address comments

* get rid of some TODOs and refine the route

* Multi-metric display on model bias screen (#1273) (#1349)

* Enhancements to model bias screen

  * Display of multiple bias charts simultaneously

  * Multi-select component, allowing free text, or select-from-list selection of chartst to display

  * Ability to collapse / expand individual charts

  * User selectable refresh rates of chart data

  * Chart selection and open / closed status is persisted to session cache for life of user's browser session

Display user defined threshold values on charts (#1163)

  * Clean up of bias chart logic

  * Displays thresholds chosen by user, or defaults if none.

  * Improves domain and threshold calculation based on user values or defaults

* Fix metrics submission issue and handle errors (#1378)

* Fix metrics submission issue and handle errors

* fix lint issue

* use error handler on GET functions

* Default and restrict threshold, add tooltips, default duplicate name and set feature flag (#1390)

* Default and restrict threshold, add tooltips, default duplicate name and set feature flag

* fix lint

* add tooltips and dropdown descriptions

* clear data when closing configuration modal

* really solve deleting issue, make empty table view a common component and apply it everywhere

* address comments

* Minor enhancements to bias chart (#1386) (#1399)

* Adds refresh interval options that match openshift observability dashboard

* Show first chart from list, if none selected when user first navigates to bias tab

* Use search icon instead of plus for nothing selected empty state

* Fix error with calculation of 30 days constant

* Deleted charts are removed from session storage

* Fixes issue with bias charts auto-refreshing with stale data (#1403) (#1404)

* Refactor prometheus queries to remove duplication

* Fix graph not refreshing issue

* Add code review suggestions

* Add performance metrics feature flag and refactor runtime server route (#1413)

* Add performance metrics feature flag, refactor runtime server route and solve layout issues

* revert some style changes

* Model serving metrics renaming (#1421)

* Adds support for TrustyAI Operator (#1443)

* Adds support for TrustyAI Operator (#1276)

* Changes from feedback

---------

Co-authored-by: Andrew Ballantyne <[email protected]>
Co-authored-by: Andrew Ballantyne <[email protected]>
Co-authored-by: Alex Creasy <[email protected]>
Co-authored-by: Alex Creasy <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@alexcreasy alexcreasy

Labels
feature/explainability feature/model-serving Model Serving Feature kind/bug Something isn't working priority/high Important issue that needs to be resolved asap. Releases should not have too many of these.
Projects
No open projects
ODH Dashboard Planning
Archived in project
Milestone
Explainability Phase 1
Development

Successfully merging a pull request may close this issue.

Fixes issue with bias charts auto-refreshing with stale data (#1403) alexcreasy/odh-dashboard
1 participant
@alexcreasy