Completed cumulativeSum

[mei1127]

@BruceDai @huningxin @fdwr @miaobin PTAL, thanks！

[BruceDai]

Thanks @mei1127 !
I left some comments. Please take a look, thanks!

[BruceDai]

The axis is of unsigned long, so its range is in [0, rank). Please modify this check and delete the tests with negative axis, thanks.

[mei1127]

done

[fdwr]

Yeah, we generally want WebNN to be more explicit, resolving these kinds of user-facing API conveniences (like negative numbers and output shape rounding) into lower-level concrete values, like the actual axis and specific output shape.

[BruceDai]

Suggested change

	const axis=0;
	const axis = 0;

[mei1127]

done

[BruceDai]

Suggested change

	export function cumulativeSum(input, axis, {exclusive = 0, reverse = 0} = {}) {
	export function cumulativeSum(input, axis, {exclusive = false, reverse = false} = {}) {

Refer to https://chromium-review.googlesource.com/c/chromium/src/+/5845069/9/third_party/blink/renderer/modules/ml/webnn/ml_graph_builder.idl#66

dictionary MLCumulativeSumOptions : MLOperatorOptions {
    boolean exclusive = false;
    boolean reversed = false;
};

[mei1127]

done

[fdwr]

Thanks May for adding this. Some thoughts 💭🧠.

[fdwr]

Suggested change

	const outputShape = [...inputShape];
	const outputShape = inputShape;

Do we need the [... ], since outputShape will always equal inputShape anyway (no modification)?

[mei1127]

done

[fdwr]

request loc -> location (whole words policy helps readability for others). Same for inputLocation and outputLocation.

[mei1127]

done

[fdwr]

Since we are guaranteed this loop will end (given step and end are selected accordingly above, which they are), this...

reverse ? i > end : i < end

...could just be...

i != end

Alternately (and probably clearer) we could make i always be relative to the output offset and use <, computing the input coordinate using the step.

  for (let i = 0; i < elementCountAlongAxis; ++i) {
    inputLocation[axis] = inputElementStart + i * inputElementStep;

[mei1127]

done

[fdwr]

Hmm, could just combine these two.

    if (!Number.isInteger(axis) || axis < 0 || axis >= rank) {
      throw new Error(`The axis ${axis} must be an unsigned integer in the interval [0, ${rank}).`);
    }

[mei1127]

done

[fdwr]

These three values never change and can be computed once outside the loop.

[fdwr]

It appears this nested for loop repeats the same computation for the same output axial sliver, overwriting the same output elements multiple times. I recommend skipping it if it's already computed in an earlier loop. e.g.

const elementStart = reverse ? elementCountAlongAxis - 1 : 0;
const elementStep = reverse ? -1 : 1;

for (let outputIndex = 0; outputIndex < totalElements; ++outputIndex) {
  let location = output.locationFromIndex(outputIndex);
  if (location[axis] > 0) {
    continue; // No need to compute this axis again, since it was already done.
  }

  let cumulativeSumValue = 0;

  // Compute the accumulated sum along this entire axis.
  for (let i = 0; i < elementCountAlongAxis; ++i) {
    location[axis] = elementStart + i * elementStep;

    const inputValue = input.getValueByLocation(inputLocation);
    const oldCumulativeSumValue = cumulativeSumValue;
    cumulativeSumValue += inputValue;
    const outputValue = exclusive ? oldCumulativeSumValue : cumulativeSumValue;
    output.setValueByLocation(outputLocation, outputValue);
  }
}

[mei1127]

done

[fdwr]

I'm still seeing the doubly nested loop such that the inner loop redundantly overwrites the previous loop's output (which will be identical each time for a given summation sliver). Notice the continue statement above, since we only need to compute the sum (inner loop) along the axis once for each distinct sliver (outer loop). Also, I since realized that the input and the output locations are always identical, meaning we can fold them into a single location variable. Updated above accordingly.

[fdwr]

Two more requests, and then it should be good. Thanks.

[fdwr]

I'm still seeing the doubly nested loop such that the inner loop redundantly overwrites the previous loop's output (which will be identical each time for a given summation sliver). Notice the continue statement above, since we only need to compute the sum (inner loop) along the axis once for each distinct sliver (outer loop). Also, I since realized that the input and the output locations are always identical, meaning we can fold them into a single location variable. Updated above accordingly.

[fdwr]

Suggested change

	inputLocation[axis]=idx;
	inputLocation[axis] = index;

Request whole words and spacing.

[mei1127]

Thanks for your advice!