keras-team / keras-core

A multi-backend implementation of the Keras API, with support for TensorFlow, JAX, and PyTorch.
Apache License 2.0
1.27k stars 117 forks source link

Add a check for class_id argument #796

Closed Frightera closed 1 year ago

Frightera commented 1 year ago

Consider the case:

import keras_core
import numpy as np

label = np.random.randint(0, high = 2, size = 100)

keras_core.metrics.Precision(class_id = 20)(label, label)

If one of the inputs are 1D and class_id is provided, the code will still execute without any errors, but it seems like the results may not be what we expect. It effectively reduces the inputs to a single value right now.

So I added a check before using class_id 🤔 or is this something intended? If not, the tests need to be updated aswell.

fchollet commented 1 year ago

Thanks for the PR. It seems this is causing some tests to fail: https://github.com/keras-team/keras-core/actions/runs/5986373040/job/16239435975?pr=796

Frightera commented 1 year ago

Hi @fchollet,

Checked the tests actually:

https://github.com/keras-team/keras-core/blob/3638a4db6428f6dd90bd540d423fe5d7efd82181/keras_core/metrics/confusion_metrics_test.py#L657-L664

Is this a correct usage of class_id in this case? The test uses a binary classification setting and passing class_id as 2. I thought class_id was valid for multiclass classification.

The other tests follow the same structure.

fchollet commented 1 year ago

Is this a correct usage of class_id in this case? The test uses a binary classification setting and passing class_id as 2. I thought class_id was valid for multiclass classification.

It's not -- the tests need to be edited.

Frightera commented 1 year ago

OK, I'll update the related tests in a few days.

Frightera commented 1 year ago

Hi @fchollet,

I updated the tests, can you review the PR now?

Thanks.