Show latest, minimum and maximum metric values on metric page #5574

adamreeve · 2022-04-01T03:00:10Z

What changes are proposed in this pull request?

Displays a table of the latest, minimum and maximum metric values in the metric view, as discussed in #5517. This partly addresses #4750. This handles the case when the metric view is used to show data for a single run and when comparing multiple runs.

Single run

Multiple runs

Multiple runs and multiple metrics

How is this patch tested?

New unit tests added.

Does this PR change the documentation?

No. You can skip the rest of this section.
Yes. Make sure the changed pages / sections render correctly by following the steps below.

Check the status of the ci/circleci: build_doc check. If it's successful, proceed to the
next step, otherwise fix it.
Click Details on the right to open the job page of CircleCI.
Click the Artifacts tab.
Click docs/build/html/index.html.
Find the changed pages / sections and make sure they render correctly.

Release Notes

Is this a user-facing change?

No. You can skip the rest of this section.
Yes. Give a description of this change to be included in the release notes for MLflow users.

The metric view now includes a table showing the latest, minimum and maximum metric values for runs.

What component(s), interfaces, languages, and integrations does this PR affect?

Components

Interface

area/uiux: Front-end, user experience, plotting, JavaScript, JavaScript dev server
area/docker: Docker use across MLflow's components, such as MLflow Projects and MLflow Models
area/sqlalchemy: Use of SQLAlchemy in the Tracking Service or Model Registry
area/windows: Windows support

Language

language/r: R APIs and clients
language/java: Java APIs and clients
language/new: Proposals for new client languages

Integrations

integrations/azure: Azure and Azure ML integrations
integrations/sagemaker: SageMaker integrations
integrations/databricks: Databricks integrations

How should the PR be classified in the release notes? Choose one:

rn/breaking-change - The PR will be mentioned in the "Breaking Changes" section
rn/none - No description will be included. The PR will be mentioned only by the PR number in the "Small Bugfixes and Documentation Updates" section
rn/feature - A new user-facing feature worth mentioning in the release notes
rn/bug-fix - A user-facing bug fix worth mentioning in the release notes
rn/documentation - A user-facing documentation change worth mentioning in the release notes

Signed-off-by: Adam Reeve <adreeve@gmail.com>

dbczumar

@adamreeve This is really awesome! Tried it out locally and it works great! I have a few small suggestions; let me know what you think of them:

Can we turn the Run <id> entries in the table into links that users can click on?
Can we display the step associated with each metric min / max / latest value in parentheses?

E.g.

Latest            |            Max      |               ....

0.355 (step=17).  |     0.4 (step=15)   |               ....

Can we make the table columns sortable so that users can easily identify the run with the smallest or largest latest / max / min value for a particular metric?
Can we make the tables fixed height and scrollable to conserve page space when there are many metrics being plotted?

Looking forward to shipping this! Thanks so much for your contribution!

@sunishsheth2009 Can you take a look as well and provide feedback?

Signed-off-by: Adam Reeve <adreeve@gmail.com>

adamreeve · 2022-04-07T05:27:02Z

Thanks for the feedback @dbczumar, all good suggestions and I've had a go at addressing those points. Making the tables fixed height was a bit awkward and meant I had to make the column widths fixed sizes, but I think that's okay and works well now. I also allowed sorting by the run or metric name for completeness, but I'm not sure if that would be used much.

Related to not taking up too much space, I'm also interested in being able to show multiple separate plots on the metric page (#5397 (comment)), so if that's something that might be accepted I'd have to think about how it fits in with this change, eg. making the summary table views collapsible, or maybe having the tables separate to the plots and being able to control the set of metrics in tables separate to the plots. Probably not worth worrying about that too much right now though, I can address that if and when it's needed.

dbczumar

@adamreeve Apologies for the delay. This is fantastic! Just a couple of tiny alignment / titling things, then this is ready to go from my perspective. Thank you for your contribution!

When there's only one metric selected, can we still show the title for it on the associated table?
I noticed that the width of the runs column causes certain run IDs to overflow on to the next line; can we expand the column width slightly?

@sunishsheth2009 Can you take a look from a code correctness / frontend best practices perspective?

sunishsheth2009

LGTM.

sunishsheth2009 · 2022-04-12T22:54:56Z

mlflow/server/js/src/experiment-tracking/components/MetricsPlotPanel.test.js

@@ -414,9 +425,8 @@ describe('unit tests', () => {
  });
  test('should render the number of completed runs correctly', () => {


Can we add a test to verify that the metric summary table is rendered as expected in the metric plot panel component?

Yep I've added a test for this now

sunishsheth2009 · 2022-04-12T23:10:54Z

mlflow/server/js/src/experiment-tracking/components/MetricsSummaryTable.js

+    });
+  }
+
+  dataColumns() {


Is there a reason that this needs to be a function rather than just a const?

It depends on this.props.intl for formatting column titles, so I think it needs to be a function?

Signed-off-by: Adam Reeve <adreeve@gmail.com>

dbczumar

LGTM! Thanks so much, @adamreeve !

@sunishsheth2009 Can you give this one more look so that we can get it merged?

sunishsheth2009

LGTM

…#5574) * Show latest, minimum and maximum metric values on metric page Signed-off-by: Adam Reeve <adreeve@gmail.com> * Consistent behaviour when no metrics selected Signed-off-by: Adam Reeve <adreeve@gmail.com> * Show corresponding step numbers next to metric values Signed-off-by: Adam Reeve <adreeve@gmail.com> * Link to runs when showing metrics for multiple runs Signed-off-by: Adam Reeve <adreeve@gmail.com> * Make metric summary columns sortable Signed-off-by: Adam Reeve <adreeve@gmail.com> * Fix metric table height and make it scrollable Signed-off-by: Adam Reeve <adreeve@gmail.com> * Fix after merge: Handle multiple experiment ids in metrics plot panel Signed-off-by: Adam Reeve <adreeve@gmail.com> * Test for presence of run links Signed-off-by: Adam Reeve <adreeve@gmail.com> * Always show metric name header Signed-off-by: Adam Reeve <adreeve@gmail.com> * Bump header column width to avoid run ids overflowing Signed-off-by: Adam Reeve <adreeve@gmail.com> * Test MetricsSummaryTable is rendered by MetricsPlotPanel Signed-off-by: Adam Reeve <adreeve@gmail.com>

adamreeve added 2 commits April 1, 2022 15:36

Show latest, minimum and maximum metric values on metric page

f9f4844

Signed-off-by: Adam Reeve <adreeve@gmail.com>

Consistent behaviour when no metrics selected

31096d4

Signed-off-by: Adam Reeve <adreeve@gmail.com>

github-actions bot added area/uiux Front-end, user experience, plotting, JavaScript, JavaScript dev server rn/feature Mention under Features in Changelogs. labels Apr 1, 2022

adamreeve mentioned this pull request Apr 1, 2022

Show minimum and maximum metric values in the run view UI #5517

Closed

29 tasks

dbczumar requested a review from sunishsheth2009 April 1, 2022 04:47

dbczumar reviewed Apr 6, 2022

View reviewed changes

adamreeve added 7 commits April 7, 2022 15:52

Show corresponding step numbers next to metric values

87bfc23

Signed-off-by: Adam Reeve <adreeve@gmail.com>

Link to runs when showing metrics for multiple runs

c0a3b92

Signed-off-by: Adam Reeve <adreeve@gmail.com>

Make metric summary columns sortable

68fa4be

Signed-off-by: Adam Reeve <adreeve@gmail.com>

Fix metric table height and make it scrollable

445214d

Signed-off-by: Adam Reeve <adreeve@gmail.com>

Merge branch 'master' into metric_view_min_max

d44cb1d

Signed-off-by: Adam Reeve <adreeve@gmail.com>

Fix after merge: Handle multiple experiment ids in metrics plot panel

b5cc19b

Signed-off-by: Adam Reeve <adreeve@gmail.com>

Test for presence of run links

6d424d4

Signed-off-by: Adam Reeve <adreeve@gmail.com>

dbczumar reviewed Apr 12, 2022

View reviewed changes

sunishsheth2009 reviewed Apr 12, 2022

View reviewed changes

adamreeve added 3 commits April 13, 2022 13:28

Always show metric name header

4973bd4

Signed-off-by: Adam Reeve <adreeve@gmail.com>

Bump header column width to avoid run ids overflowing

86c77ec

Signed-off-by: Adam Reeve <adreeve@gmail.com>

Test MetricsSummaryTable is rendered by MetricsPlotPanel

3a8485a

Signed-off-by: Adam Reeve <adreeve@gmail.com>

dbczumar approved these changes Apr 20, 2022

View reviewed changes

sunishsheth2009 approved these changes Apr 22, 2022

View reviewed changes

dbczumar merged commit 37c6783 into mlflow:master Apr 28, 2022

adamreeve deleted the metric_view_min_max branch May 8, 2022 22:05

dbczumar mentioned this pull request Jun 2, 2022

Feature request: in the ui select minimum and maximum of a metric #1422

Closed

harupy mentioned this pull request Jun 6, 2022

[FR] Display max/min value of a metric in the experiments UI #4750

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show latest, minimum and maximum metric values on metric page #5574

Show latest, minimum and maximum metric values on metric page #5574

adamreeve commented Apr 1, 2022

dbczumar left a comment

adamreeve commented Apr 7, 2022

dbczumar left a comment •

edited

Loading

sunishsheth2009 left a comment

sunishsheth2009 Apr 12, 2022 •

edited

Loading

adamreeve Apr 13, 2022

sunishsheth2009 Apr 12, 2022

adamreeve Apr 12, 2022

dbczumar left a comment

sunishsheth2009 left a comment

		@@ -414,9 +425,8 @@ describe('unit tests', () => {
		});
		test('should render the number of completed runs correctly', () => {

Show latest, minimum and maximum metric values on metric page #5574

Show latest, minimum and maximum metric values on metric page #5574

Conversation

adamreeve commented Apr 1, 2022

What changes are proposed in this pull request?

Single run

Multiple runs

Multiple runs and multiple metrics

How is this patch tested?

Does this PR change the documentation?

Release Notes

Is this a user-facing change?

What component(s), interfaces, languages, and integrations does this PR affect?

How should the PR be classified in the release notes? Choose one:

dbczumar left a comment

Choose a reason for hiding this comment

adamreeve commented Apr 7, 2022

dbczumar left a comment • edited Loading

Choose a reason for hiding this comment

sunishsheth2009 left a comment

Choose a reason for hiding this comment

sunishsheth2009 Apr 12, 2022 • edited Loading

Choose a reason for hiding this comment

adamreeve Apr 13, 2022

Choose a reason for hiding this comment

sunishsheth2009 Apr 12, 2022

Choose a reason for hiding this comment

adamreeve Apr 12, 2022

Choose a reason for hiding this comment

dbczumar left a comment

Choose a reason for hiding this comment

sunishsheth2009 left a comment

Choose a reason for hiding this comment

dbczumar left a comment •

edited

Loading

sunishsheth2009 Apr 12, 2022 •

edited

Loading