Skip to content

Classification metrics#72

Open
jan-www wants to merge 2 commits intomainfrom
metrics
Open

Classification metrics#72
jan-www wants to merge 2 commits intomainfrom
metrics

Conversation

@jan-www
Copy link
Copy Markdown
Collaborator

@jan-www jan-www commented Nov 1, 2021

No description provided.

Comment thread src/preprocess/encoder.ts Outdated
Comment thread src/metrics/classifier.ts Outdated
Comment thread src/metrics/classifier.ts
return [ yTrueTensor as Tensor1D, yPredTensor as Tensor1D, yTrueCount ];
};

export const accuracyScore = (yTrue: Tensor | string[] | number[], yPred: Tensor | string[] | number[]): number => {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

Comment thread src/metrics/classifier.ts
await labelEncoder.init(concat([ yTrueTensor, yPredTensor ]));
const yTrueEncode = await labelEncoder.encode(yTrueTensor);
const yPredEncode = await labelEncoder.encode(yPredTensor);
const numClasses = labelEncoder.categories.shape[0];
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const numClasses = labelEncoder.categories.shape[0];
const numOfClasses = labelEncoder.categories.shape[0];

Comment thread src/metrics/classifier.ts Outdated
Comment thread src/metrics/classifier.ts Outdated
Comment thread src/metrics/classifier.ts Outdated
const averageF1 = average == 'weighted' ? mul(f1s, weights).dataSync()[0] : divNoNan(sum(f1s), numClasses).dataSync()[0];
return {
precisions: precisions,
recalls: recalls,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
recalls: recalls,
recalls,

Comment thread src/metrics/classifier.ts Outdated
Comment thread src/preprocess/encoder.ts Outdated
@@ -1,4 +1,4 @@
import { Tensor, unique, oneHot, cast, tensor, argMax, reshape, slice, stack, sub, squeeze, greaterEqual, topk } from "@tensorflow/tfjs-core";
import { Tensor, unique, oneHot, cast, tensor, argMax, reshape, slice, stack, sub, squeeze, greaterEqual, topk, Tensor1D, tidy } from "@tensorflow/tfjs-core";
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
import { Tensor, unique, oneHot, cast, tensor, argMax, reshape, slice, stack, sub, squeeze, greaterEqual, topk, Tensor1D, tidy } from "@tensorflow/tfjs-core";
import { Tensor, unique, oneHot, cast, tensor, argMax, reshape, slice, stack, sub, squeeze, greaterEqual, topk, Tensor1D, tidy } from '@tensorflow/tfjs-core';

Comment thread src/preprocess/encoder.ts
}
this.cateMap = cateMap;
}
abstract encode(x: Tensor | number[] | string[]): Promise<Tensor>;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we use TensorLike1D instead of the type expression?

Comment thread src/preprocess/encoder.ts Outdated
export class OneHotEncoder {
public categories: Tensor;
public cateMap: CateMap;
export class OneHotEncoder extends EncoderBase{
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
export class OneHotEncoder extends EncoderBase{
export class OneHotEncoder extends EncoderBase {

Comment thread src/preprocess/encoder.ts Outdated
}
}

export class LabelEncoder extends EncoderBase{
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
export class LabelEncoder extends EncoderBase{
export class LabelEncoder extends EncoderBase {

Comment thread src/preprocess/encoder.ts
*/
public async encode(x: Tensor | number[] | string[]): Promise<Tensor> {
if (!this.categories) {
throw TypeError('Please init encoder using init()');
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
throw TypeError('Please init encoder using init()');
throw new TypeError('Please initialize an encoder using `init()`');

@jan-www jan-www changed the title Metrics and Covariance Classification metrics Nov 1, 2021
Comment thread src/preprocess/encoder.ts Outdated
throw TypeError('Please init encoder using init()');
}
const xTensor = checkArray(x, 'any', 1);
const xData = await xTensor.dataSync();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const xData = await xTensor.dataSync();
const xData = await xTensor.data();

Comment thread src/preprocess/encoder.ts
const xTensor = checkArray(x, 'any', 1);
const xData = await xTensor.dataSync();
xTensor.dispose();
return tensor(xData.map((d: number|string) => this.cateMap[d]));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return tensor(xData.map((d: number|string) => this.cateMap[d]));
return tensor(xData.map((d) => this.cateMap[d]));

Is the type required?

Comment thread src/preprocess/encoder.ts
*/
public async decode(x: Tensor | number[]): Promise<Tensor> {
if (!this.categories) {
throw TypeError('Please init encoder using init()');
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
throw TypeError('Please init encoder using init()');
throw new TypeError('Please initialize an encoder using `init()`');

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about implementing this in the base class?

Comment thread test/node/preprocess/encoder.ts Outdated
const encoder = new LabelEncoder();
await encoder.init(x);
const xEncode = await encoder.encode(x);
assert.deepEqual(xEncode.dataSync(), xLabelEncode.dataSync());
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assert.deepEqual(xEncode.dataSync(), xLabelEncode.dataSync());
assert.deepEqual(await xEncode.data(), await xLabelEncode.data());

There is no need to use dataSync() here.

Comment thread test/node/preprocess/encoder.ts Outdated
it('encode', async () => {
const encoder = new LabelEncoder();
await encoder.init(x);
const xEncode = await encoder.encode(x);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The xEncode is declared at line#9, how about a new name?

Comment thread test/node/preprocess/encoder.ts Outdated
const encoder = new LabelEncoder();
await encoder.init(x);
const xDecode = await encoder.decode(xLabelEncode);
assert.deepEqual(x, xDecode.dataSync() as any);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto, use await *.data() instead of dataSync().

Comment thread src/linalg/utils.ts
Comment thread src/metrics/classifier.ts Outdated
@@ -1,15 +1,77 @@
import { Tensor, equal, sum, div } from '@tensorflow/tfjs-core';
import { Tensor, equal, sum, div, math, Tensor1D, divNoNan, concat, mul, add, cast } from '@tensorflow/tfjs-core';
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
import { Tensor, equal, sum, div, math, Tensor1D, divNoNan, concat, mul, add, cast } from '@tensorflow/tfjs-core';
import { Tensor, Tensor1D, equal, sum, div, math, divNoNan, concat, mul, add, cast } from '@tensorflow/tfjs-core';

Comment thread src/metrics/classifier.ts Outdated
* @param yPred predicted labels
* @returns classification report object, the struct of report will be like following
*/
export const classificationReport = async(yTrue: Tensor | string[] | number[], yPred: Tensor | string[] | number[], average: ClassificationAverageTypes = 'weighted'): Promise<ClassificationReport> => {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And this line is too long(over 80 chars).

Comment thread src/metrics/classifier.ts Outdated
* @param yTrue true labels
* @param yPred predicted labels
*/
export const checkSameLength = (yTrue: Tensor | string[] | number[], yPred: Tensor | string[] | number[]): [ Tensor1D, Tensor1D, number ] => {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function must be moved to utils/validation.

Comment thread src/metrics/classifier.ts Outdated
const f1s = divNoNan(divNoNan(mul(precisions, recalls), add(precisions, recalls)), 2);
const accuracy = accuracyScore(yTrue, yPred);
const weights = divNoNan(sum(confusionMatrix, 0), sum(confusionMatrix));
const averagePrecision = average == 'weighted' ? mul(precisions, weights).dataSync()[0] : divNoNan(sum(precisions), numClasses).dataSync()[0];
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const averagePrecision = average == 'weighted' ? mul(precisions, weights).dataSync()[0] : divNoNan(sum(precisions), numClasses).dataSync()[0];
const averagePrecision = average === 'weighted' ? mul(precisions, weights).dataSync()[0] : divNoNan(sum(precisions), numClasses).dataSync()[0];

Comment thread src/metrics/classifier.ts Outdated
* @param yPred predicted labels
* @returns classification report object, the struct of report will be like following
*/
export const classificationReport = async(yTrue: Tensor | string[] | number[], yPred: Tensor | string[] | number[], average: ClassificationAverageTypes = 'weighted'): Promise<ClassificationReport> => {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
export const classificationReport = async(yTrue: Tensor | string[] | number[], yPred: Tensor | string[] | number[], average: ClassificationAverageTypes = 'weighted'): Promise<ClassificationReport> => {
export const classificationReport = async (yTrue: Tensor | string[] | number[], yPred: Tensor | string[] | number[], average: ClassificationAverageTypes = 'weighted'): Promise<ClassificationReport> => {

Comment thread src/metrics/classifier.ts Outdated
const f1s = divNoNan(divNoNan(mul(precisions, recalls), add(precisions, recalls)), 2);
const accuracy = accuracyScore(yTrue, yPred);
const weights = divNoNan(sum(confusionMatrix, 0), sum(confusionMatrix));
const averagePrecision = average == 'weighted' ? mul(precisions, weights).dataSync()[0] : divNoNan(sum(precisions), numClasses).dataSync()[0];
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const averagePrecision = average == 'weighted' ? mul(precisions, weights).dataSync()[0] : divNoNan(sum(precisions), numClasses).dataSync()[0];
const averagePrecision = await (average == 'weighted' ? mul(precisions, weights).data() : divNoNan(sum(precisions), numClasses).data())[0];

Comment thread src/metrics/classifier.ts Outdated
Comment thread src/metrics/classifier.ts Outdated
const weights = divNoNan(sum(confusionMatrix, 0), sum(confusionMatrix));
const averagePrecision = average == 'weighted' ? mul(precisions, weights).dataSync()[0] : divNoNan(sum(precisions), numClasses).dataSync()[0];
const averageRecall = average == 'weighted' ? mul(recalls, weights).dataSync()[0] : divNoNan(sum(recalls), numClasses).dataSync()[0];
const averageF1 = average == 'weighted' ? mul(f1s, weights).dataSync()[0] : divNoNan(sum(f1s), numClasses).dataSync()[0];
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

@yorkie
Copy link
Copy Markdown
Member

yorkie commented Jan 31, 2022

  1. Firstly rebase your change based on the latest main branch, because there are some commits which do not belong to this PR
  2. Squash your changes

@yorkie
Copy link
Copy Markdown
Member

yorkie commented Feb 9, 2022

There are still some comments not getting resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants