Classification metrics by jan-www · Pull Request #72 · imgcook/datacook

jan-www · 2021-11-01T03:33:37Z

No description provided.

src/preprocess/encoder.ts

src/metrics/classifier.ts

yorkie · 2021-11-01T07:32:06Z

src/metrics/classifier.ts

+  return [ yTrueTensor as Tensor1D, yPredTensor as Tensor1D, yTrueCount ];
+};
+
+export const accuracyScore = (yTrue: Tensor | string[] | number[], yPred: Tensor | string[] | number[]): number => {


yorkie · 2021-11-01T07:55:02Z

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];


Suggested change

const numClasses = labelEncoder.categories.shape[0];

const numOfClasses = labelEncoder.categories.shape[0];

src/metrics/classifier.ts

yorkie · 2021-11-01T07:58:25Z

src/metrics/classifier.ts

+  const averageF1 = average == 'weighted' ? mul(f1s, weights).dataSync()[0] : divNoNan(sum(f1s), numClasses).dataSync()[0];
+  return {
+    precisions: precisions,
+    recalls: recalls,


Suggested change

recalls: recalls,

recalls,

src/metrics/classifier.ts

yorkie · 2021-11-01T07:59:19Z

src/preprocess/encoder.ts

@@ -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";


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';

yorkie · 2021-11-01T08:03:10Z

src/preprocess/encoder.ts

+    }
+    this.cateMap = cateMap;
+  }
+  abstract encode(x: Tensor | number[] | string[]): Promise<Tensor>;


Shall we use TensorLike1D instead of the type expression?

yorkie · 2021-11-01T08:07:03Z

src/preprocess/encoder.ts

-export class OneHotEncoder {
-  public categories: Tensor;
-  public cateMap: CateMap;
+export class OneHotEncoder extends EncoderBase{


Suggested change

export class OneHotEncoder extends EncoderBase{

export class OneHotEncoder extends EncoderBase {

yorkie · 2021-11-01T08:07:52Z

src/preprocess/encoder.ts

  }
 }
+
+export class LabelEncoder extends EncoderBase{


Suggested change

export class LabelEncoder extends EncoderBase{

export class LabelEncoder extends EncoderBase {

yorkie · 2021-11-01T08:08:42Z

src/preprocess/encoder.ts

+   */
+  public async encode(x: Tensor | number[] | string[]): Promise<Tensor> {
+    if (!this.categories) {
+      throw TypeError('Please init encoder using init()');


Suggested change

throw TypeError('Please init encoder using init()');

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

yorkie · 2021-11-01T08:09:31Z

src/preprocess/encoder.ts

+      throw TypeError('Please init encoder using init()');
+    }
+    const xTensor = checkArray(x, 'any', 1);
+    const xData = await xTensor.dataSync();


Suggested change

const xData = await xTensor.dataSync();

const xData = await xTensor.data();

yorkie · 2021-11-01T08:10:40Z

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]));


Suggested change

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

return tensor(xData.map((d) => this.cateMap[d]));

Is the type required?

yorkie · 2021-11-01T08:11:04Z

src/preprocess/encoder.ts

+   */
+  public async decode(x: Tensor | number[]): Promise<Tensor> {
+    if (!this.categories) {
+      throw TypeError('Please init encoder using init()');


Suggested change

throw TypeError('Please init encoder using init()');

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

How about implementing this in the base class?

yorkie · 2021-11-01T08:15:29Z

test/node/preprocess/encoder.ts

+    const encoder = new LabelEncoder();
+    await encoder.init(x);
+    const xEncode = await encoder.encode(x);
+    assert.deepEqual(xEncode.dataSync(), xLabelEncode.dataSync());


Suggested change

assert.deepEqual(xEncode.dataSync(), xLabelEncode.dataSync());

assert.deepEqual(await xEncode.data(), await xLabelEncode.data());

There is no need to use dataSync() here.

yorkie · 2021-11-01T08:18:16Z

test/node/preprocess/encoder.ts

+  it('encode', async () => {
+    const encoder = new LabelEncoder();
+    await encoder.init(x);
+    const xEncode = await encoder.encode(x);


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

yorkie · 2021-11-01T08:18:45Z

test/node/preprocess/encoder.ts

+    const encoder = new LabelEncoder();
+    await encoder.init(x);
+    const xDecode = await encoder.decode(xLabelEncode);
+    assert.deepEqual(x, xDecode.dataSync() as any);


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

src/linalg/utils.ts

yorkie · 2021-11-01T08:22:56Z

src/metrics/classifier.ts

@@ -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';


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';

yorkie · 2021-11-01T08:24:15Z

src/metrics/classifier.ts

+ * @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> => {


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

yorkie · 2021-11-01T08:26:23Z

src/metrics/classifier.ts

+ * @param yTrue true labels
+ * @param yPred predicted labels
+ */
+export const checkSameLength = (yTrue: Tensor | string[] | number[], yPred: Tensor | string[] | number[]): [ Tensor1D, Tensor1D, number ] => {


This function must be moved to utils/validation.

yorkie · 2021-11-01T08:27:36Z

src/metrics/classifier.ts

+  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];


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];

yorkie · 2021-11-01T08:27:52Z

src/metrics/classifier.ts

+ * @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> => {


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> => {

yorkie · 2021-11-01T08:28:45Z

src/metrics/classifier.ts

+  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];


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];

src/metrics/classifier.ts

yorkie · 2021-11-01T08:29:10Z

src/metrics/classifier.ts

+  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];


yorkie · 2022-01-31T10:48:38Z

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

yorkie · 2022-02-09T15:15:24Z

There are still some comments not getting resolved.

jan-www requested review from FeelyChau, rickycao-qy and yorkie November 1, 2021 07:02

rickycao-qy requested changes Nov 1, 2021

View reviewed changes

src/preprocess/encoder.ts Outdated Show resolved Hide resolved