Fix contour plot on run comparison page

[harupy]

Signed-off-by: harupy 17039389+harupy@users.noreply.github.com

What changes are proposed in this pull request?

• Fixes [BUG] Contour Plot render broken since 1.8 #3503
• It's still WIP.

How is this patch tested?

• Manually verify the plot works correctly
• Unit tests

Before (master branch)

contour-issue-before.mov

• plotly tries to draw the contour plot with p1, p2, and m1, but fails because p1 has a single unique value: 0 (see the error message on the console). The contour plot seems to require at least two data points on each axis.
• plotly keeps failing to draw the plot even if we pass valid data.
• Note this issue occurs when we use scattergl (on this line). The plot gets drawn successfully with scatter. I haven't taken a look at the internal implementation of plotly, but it looks like using scattergl changes how plotly updates the plot state.

After

contour-issue-after.mov

• The fix is to avoid drawing the plot if we know the data to plot doesn't enough data points.
• Showing a warning message is more helpful than showing an empty plot?

Code to generate test data

import mlflow

with mlflow.start_run():
    mlflow.log_param("p1", 0)
    mlflow.log_param("p2", 0.14427203773889796)
    mlflow.log_param("m1", 0)
    mlflow.log_metric("m2", 0.815)

with mlflow.start_run():
    mlflow.log_param("p1", 0)
    mlflow.log_param("p2", 0.17649088560647586)
    mlflow.log_param("m1", 0)
    mlflow.log_metric("m2", 0.269)

with mlflow.start_run():
    mlflow.log_param("p1", 0)
    mlflow.log_param("p2", 0.5324973999849046)
    mlflow.log_param("m1", 0)
    mlflow.log_metric("m2", 0.414)

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.

(Details in 1-2 sentences. You can just refer to another PR with a description if this PR is part of a larger change.)

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

Components

• area/artifacts: Artifact stores and artifact logging
• area/build: Build and test infrastructure for MLflow
• area/docs: MLflow documentation pages
• area/examples: Example code
• area/model-registry: Model Registry service, APIs, and the fluent client calls for Model Registry
• area/models: MLmodel format, model serialization/deserialization, flavors
• area/projects: MLproject format, project running backends
• area/scoring: MLflow Model server, model deployment tools, Spark UDFs
• area/server-infra: MLflow Tracking server backend
• area/tracking: Tracking Service, tracking client APIs, autologging

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

[harupy]

Suggested change

	<div>X or Y axis doesn't have enough unique data points to draw the contour plot</div>
	<div>The contour plot requires at least two unique data points on the X and Y axes.</div>

Does this sound clearer?

[jinzhang21]

How about splitting the if-clause into 2 to improve the error message to indicate which axis it is?

[harupy]

Thanks for the suggestion! Sounds good so the implementation should look like this, right?

Suggested change

	<div>X or Y axis doesn't have enough unique data points to draw the contour plot</div>
	const invalidAxes = [];
	if (xsSorted.length < 2) {invalidAxes.push("X")}
	if (ysSorted.length < 2) {invalidAxes.push("Y")}
	if (invalidAxes.length > 0) {
	const message = (
	invalidAxes.length > 1 ?
	`The ${invalidAxes.join("and")} axes don't` :
	`The ${invalidAxes[0]} axis doesn't` :
	)
	return <div>{`${message}` enough data points to render the contour plot.}</div>
	}

[jinzhang21]

Nice!

[jinzhang21]

Which lines in this moved function is changed? Also why makes it a LazyPlot?

[harupy]

Thanks for the comment. I just added the if clause right above to avoid attempting to render the plot with invalid data.

[harupy]

Also why makes it a LazyPlot?

It's because the original implementation uses LazyPlot?

[jinzhang21]

How about splitting the if-clause into 2 to improve the error message to indicate which axis it is?