Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement argMax and argMin #62

Merged
merged 6 commits into from
Dec 18, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 66 additions & 0 deletions src/arg_max_min.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
'use strict';

import {Tensor, sizeOfShape} from './lib/tensor.js';
import {reduceMax, reduceMin, selectValuesToReduce} from './reduce.js';
import {squeeze} from './squeeze.js';

/**
* Get the index location of the minimum or maxmium values of all the input values along the axes.
* @param {Tensor} input
* @param {Function} reduceFunc
* @param {MLArgMinMaxOptions} [options]
* @return {Tensor}
*/
export function argMaxMin(
input,
reduceFunc,
{
axes = null,
keepDimensions = false,
selectLastIndex = false,
} = {}) {
const inpAxes = axes ?? new Array(input.rank).fill(0).map((_, i) => i);
BruceDai marked this conversation as resolved.
Show resolved Hide resolved
BruceDai marked this conversation as resolved.
Show resolved Hide resolved
Copy link

Choose a reason for hiding this comment

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

Regarding axes ??, I'm not that familiar with the falsesyness of JS evaluation. If you pass axes = [], will that correctly use the explicit empty axes (output shape is then the same size as the input, and there is a 1:1 mapping of output indices) rather than generate full reduction?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

axes = []

It's so very special, it's neither null nor a sequence having values in the range [0, N-1]. Is there any model using this axes?

According to https://webmachinelearning.github.io/webnn/#dom-mlargminmaxoptions-axes:

axes, of type sequence<unsigned long>, defaulting to null
A sequence of [unsigned long](https://webidl.spec.whatwg.org/#idl-unsigned-long). The dimensions to reduce. The values in the sequence must be in the range [0, N-1] where N is the [rank](https://webmachinelearning.github.io/webnn/#mloperand-rank) of the input tensor. If not present, all dimensions are reduced.

Copy link

@fdwr fdwr Dec 15, 2023

Choose a reason for hiding this comment

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

My argument is mainly driven by robustness plus consistency with similar behavior for the reduction operators when passed explicit [] axes. You probably won't find a model unless you look really hard, but some model generation parameters could programmatically produce it; and we see that in related reduction operators like ReduceSum that explicit empty axes [] was important enough that ONNX specifically added the noop_with_empty_axes attribute later https://onnx.ai/onnx/operators/onnx__ReduceSum.html. Speaking of that, do we have any explicit axes = [] cases in reduceSum?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Last we also missed considering this axes = [] cases in reduceSum.
Since current Spec is lack of explanation for axes = [], I will open an issue for discussing it on WG.

Copy link

Choose a reason for hiding this comment

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

I will open an issue for discussing it on WG.

Alrighty, and I will add my clear expectation of what should happen in the nop case 😉 (especially if WebNN is to support existing frameworks behavior where [] means nop).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link

Choose a reason for hiding this comment

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

That was fast! I tried ONNX and TF:

x = tf.constant([[0,100],[200,300]], dtype=tf.float32)
y = tf.math.reduce_sum(x, axis=[]) # https://www.tensorflow.org/api_docs/python/tf/math/reduce_sum
print("value:", y)
print("shape:", y.shape)

# Prints:
# value: tf.Tensor(
# [[  0. 100.]
#  [200. 300.]], shape=(2, 2), dtype=float32)
# shape: (2, 2)

(will add that to the issue...)

Copy link

Choose a reason for hiding this comment

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

Do you want to wait for the spec discussion or proceed and react later accordingly? (I don't mind either way, and you might not get a reply for the next 2 weeks...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. I will follow your guide to support axes=[] here.

Copy link

Choose a reason for hiding this comment

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

p.s. Appears that the Chromium implementation supports explicit axes = [] gracefully:
https://chromium-review.googlesource.com/c/chromium/src/+/5125652

const outputShape = input.shape.slice();

for (let i = 0; i < inpAxes.length; ++i) {
outputShape[inpAxes[i]] = 1;
}

let output = new Tensor(outputShape);
const tensor = reduceFunc(input, {axes: inpAxes, keepDimensions: true});

for (let outputIndex = 0; outputIndex < sizeOfShape(outputShape); ++outputIndex) {
const value = tensor.getValueByIndex(outputIndex);
const inputLocation = output.locationFromIndex(outputIndex);
const selectedArray = selectValuesToReduce(input, inpAxes, inputLocation);
const index =
selectLastIndex ? selectedArray.lastIndexOf(value) : selectedArray.indexOf(value);
output.setValueByIndex(outputIndex, index);
}

if (!keepDimensions) {
output = squeeze(output, {axes});
}

return output;
}

/**
* Get the index location of the maxmium values of all the input values along the axes.
* @param {Tensor} input
* @param {MLArgMinMaxOptions} [options]
* @return {Tensor}
*/
export function argMax(input, options = {}) {
return argMaxMin(input, reduceMax, options);
}

/**
* Get the index location of the minimum values of all the input values along the axes.
* @param {Tensor} input
* @param {MLArgMinMaxOptions} [options]
* @return {Tensor}
*/
export function argMin(input, options = {}) {
return argMaxMin(input, reduceMin, options);
}
2 changes: 1 addition & 1 deletion src/lib/validate-input.js
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ export function validatePool2dParams(input, _, {roundingType = 'floor'}) {
}
}

export function validateReduceParams(input, _, {axes}) {
export function validateReduceParams(input, {axes}) {
if (axes.length > input.rank) {
throw new Error(`The length ${axes.length} of axes is bigger` +
`than input rank ${input.rank}.`);
Expand Down
64 changes: 37 additions & 27 deletions src/reduce.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,40 @@ import {abs, exp, log} from './unary.js';
import {sizeOfShape, Scalar, Tensor} from './lib/tensor.js';
import {validateReduceParams} from './lib/validate-input.js';

export function selectValuesToReduce(input, axes, inputLocation) {
validateReduceParams(input, {axes});

const outputShape = input.shape.slice();
for (let i = 0; i < axes.length; ++i) {
outputShape[axes[i]] = 1;
}

// Calculate the "strides" across the reduction dimensions given in axes.
axes.sort((a, b) => a - b);
BruceDai marked this conversation as resolved.
Show resolved Hide resolved
const reduceDims = axes.map((axis) => input.shape[axis]);
const reduceElements = sizeOfShape(reduceDims);
const reduceStrides = new Array(axes.length);
reduceStrides[reduceStrides.length - 1] = 1;
Copy link

Choose a reason for hiding this comment

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

Appears this will fail on the axes = [] case, which should return exactly one element of shape []. Most of the logic already looks correct to handle that case (including sizeOfShape which will return 1).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's strange. It can work for this case without changes, my understanding is that index should be start from 0.

reduceStrides = [];
reduceStrides[-1] = 1; // it can work

But I add if condition block before this, then it never goes here for axes=[], please see the update commit, thanks.

Copy link

@fdwr fdwr Dec 16, 2023

Choose a reason for hiding this comment

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

It's strange. It can work for this case without changes
reduceStrides[-1] = 1; // it can work

(huh, what a weird language: https://stackoverflow.com/questions/58570277/adding-an-element-in-negative-index-of-an-array)

Copy link
Contributor

Choose a reason for hiding this comment

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

reduceStrides[-1] = 1 getting [-1: 1] is bad, we should avoid that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @huningxin!
I've updated commit to avoid it, please take another look, thanks.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
reduceStrides[reduceStrides.length - 1] = 1;
if (reduceStrides.length > 0) {
reduceStrides[reduceStrides.length - 1] = 1;
}

for (let i = reduceStrides.length - 2; i >= 0; --i) {
reduceStrides[i] = reduceStrides[i + 1] * reduceDims[i + 1];
}

const valuesToReduce = [];
// Find all values to reduce.
for (let reduceIndex = 0; reduceIndex < reduceElements; ++reduceIndex) {
Copy link

Choose a reason for hiding this comment

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

How about reducedElementCount? reduceElements sounds like it's actually the elements themselves (the values).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

// Calculate the input location given index of elements to reduce.
let remainingReduceIndex = reduceIndex;
for (let i = 0; i < axes.length; ++i) {
const axis = axes[i];
inputLocation[axis] = Math.floor(remainingReduceIndex / reduceStrides[i]);
remainingReduceIndex -= inputLocation[axis] * reduceStrides[i];
}
valuesToReduce.push(input.getValueByLocation(inputLocation));
}

return valuesToReduce;
}

/**
* Reduce the input along the dimensions given in axes.
* @param {Tensor} input
Expand All @@ -15,45 +49,21 @@ import {validateReduceParams} from './lib/validate-input.js';
*/
function reduce(input, reduceFunc, {keepDimensions = false, axes} = {}) {
const inpAxes = axes ?? new Array(input.rank).fill(0).map((_, i) => i);

const outputShape = input.shape.slice();
for (let i = 0; i < inpAxes.length; ++i) {
outputShape[inpAxes[i]] = 1;
}

validateReduceParams(input, reduceFunc, {keepDimensions, axes: inpAxes});

// Calculate the "strides" across the reduction dimensions given in axes.
inpAxes.sort((a, b) => a - b);
const reduceDims = inpAxes.map((axis) => input.shape[axis]);
const reduceElements = sizeOfShape(reduceDims);
const reduceStrides = new Array(inpAxes.length);
reduceStrides[reduceStrides.length - 1] = 1;
for (let i = reduceStrides.length - 2; i >= 0; --i) {
reduceStrides[i] = reduceStrides[i + 1] * reduceDims[i + 1];
}

let output = new Tensor(outputShape);
for (let outputIndex = 0; outputIndex < sizeOfShape(outputShape); ++outputIndex) {
const valuesToReduce = [];
// Find all values to reduce.
for (let reduceIndex = 0; reduceIndex < reduceElements; ++reduceIndex) {
// Calculate the input location given index of elements to reduce.
const inputLocation = output.locationFromIndex(outputIndex);
let remainingReduceIndex = reduceIndex;
for (let i = 0; i < inpAxes.length; ++i) {
const axis = inpAxes[i];
inputLocation[axis] = Math.floor(remainingReduceIndex / reduceStrides[i]);
remainingReduceIndex -= inputLocation[axis] * reduceStrides[i];
}
valuesToReduce.push(input.getValueByLocation(inputLocation));
}
const inputLocation = output.locationFromIndex(outputIndex);
const valuesToReduce = selectValuesToReduce(input, inpAxes, inputLocation);
const outputValue = valuesToReduce.reduce(reduceFunc);
output.setValueByIndex(outputIndex, outputValue);
}

if (!keepDimensions) {
output = squeeze(output);
output = squeeze(output, {axes});
}
return output;
}
Expand Down
Loading