kubeflow / pipelines

Machine Learning Pipelines for Kubeflow
https://www.kubeflow.org/docs/components/pipelines/
Apache License 2.0
3.51k stars 1.58k forks source link

[sdk] SlicedClassificationMetrics Artifact load_confusion_matrix has incorrect call #8117

Closed hollyhutson closed 2 months ago

hollyhutson commented 1 year ago

Environment

Steps to reproduce

Use a SlicedClassificationMetrics Artifact and try to call load_confusion_matrix. Internally this calls log_confusion_matrix_cell on the ClassificationMetrics object instead of log_confusion_matrix, which results in a type error: TypeError: log_confusion_matrix_cell() missing 1 required positional argument: 'value'

Expected result

Calling load_confusion_matrix on SlicedClassificationMetrics doesn't break, and correctly adds the confusion matrix to the right slice in the metadata.

Materials and Reference

Full reproducible sample:

from kfp.v2.dsl import component, Output, SlicedClassificationMetrics
from kfp.v2 import compiler
import kfp

@component()
def simple_metric_op(sliced_eval_metrics: Output[SlicedClassificationMetrics]):
    sliced_eval_metrics._sliced_metrics = {}
    sliced_eval_metrics.load_confusion_matrix('a slice', 
                                                categories=['cat1', 'cat2'],
                                                matrix=[[1,0],[2,4]]
                                                )

@kfp.dsl.pipeline(name="test-sliced-metric")
def metric_test_pipeline():
    m_op = simple_metric_op()

compiler.Compiler().compile(
    pipeline_func=metric_test_pipeline,
    package_path="evaluation_pipeline.json",
)

See the incorrect line of code here: https://github.com/kubeflow/pipelines/blob/061905b6df397c40fbcc4ffafa24d7b3b9daf439/sdk/python/kfp/components/types/artifact_types.py#L452-L464 Where self._sliced_metrics[slice].log_confusion_matrix_cell( categories, matrix) Should be self._sliced_metrics[slice].log_confusion_matrix( categories, matrix) Since self._sliced_metrics[slice] is of type ClassificationMetrics after the call to self._upsert_classification_metrics_for_slice(slice)

As an aside - _sliced_metrics is never initialised in the class, leaving it up to the user of the Artifact class, which isn't very intuitive.


Impacted by this bug? Give it a 👍.

github-actions[bot] commented 4 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions[bot] commented 2 months ago

This issue has been automatically closed because it has not had recent activity. Please comment "/reopen" to reopen it.