wgsl: Add AF Division execution tests #3074
Conversation
Adds in forwarding of ULP and division interval calls to f32 for abstract Issue gpuweb#1626
|
Previews, as seen when this build job started (f58eab5): |
This allows for removing wrapper utilities and directly use arrow functions instead.
|
Previews, as seen when this build job started (bf48a54): |
| const constants = this.constants(); | ||
| const domain_x = [this.toInterval([constants.negative.min, constants.positive.max])]; | ||
| const domain_y = | ||
| this.kind === 'f32' | ||
| this.kind === 'f32' || this.kind === 'abstract' |
There was a problem hiding this comment.
By using the same domain_y for af and f32, the division interval op will return unbounded interval for af in the wide range of normal af number that out of f32 range.
But since the spec didn't mention what to expect in that range for af, this would be OK.
There was a problem hiding this comment.
Yes. Thank you for reading this very closely.
The thinking is that we don't want WGSL to specify anything more strict than ECMAScript, and ECMAScript does not specify much in terms of ULP. On the other hand, we also don't want AbstractFloat to be worse than f32.
| ['scalar']: () => { | ||
| return FP.abstract.generateScalarPairToIntervalCases( | ||
| sparseF64Range(), | ||
| sparseF64Range(), |
There was a problem hiding this comment.
Some data point in this range may be out of domain_y of division interval operation, and result in unbounded expectation. Good thing is that this won't cause false error.
src/webgpu/util/math.ts
Outdated
| m.every(c => c.length === r), | ||
| `Unexpectedly received jagged array to map` | ||
| ); | ||
| for (let i = 0; i < c; i++) { |
There was a problem hiding this comment.
Would the following be better?
return m.every(col => col.every(el => op(el)));
src/unittests/floating_point.spec.ts
Outdated
| const minusOneULP = kMinusOneULPFunctions[p.trait]; | ||
| const minusNULP = kMinusNULPFunctions[p.trait]; | ||
| // For ULP purposes, abstract float behaves like f32, so swizzling it in. | ||
| const trait = p.trait === 'abstract' ? 'f32' : p.trait; |
There was a problem hiding this comment.
How about introducing some kind of FP mapping dictionary for ULP propose, instead of using ?: in multiple places?
Something in the file scope like
// For ULP purposes, abstract float behaves like f32, so swizzling it in.
const kFPTraitForULP = {
abstract: 'f32',
f32: 'f32',
f16: 'f16',
} as const;and then use e.g.
const constants = FP[kFPTraitForULP[p.trait]].constants();
const ULPValue = kULPErrorValue[kFPTraitForULP[p.trait]];
const plusOneULP = kPlusOneULPFunctions[kFPTraitForULP[p.trait]];
const plusNULP = kPlusNULPFunctions[kFPTraitForULP[p.trait]];
const minusOneULP = kMinusOneULPFunctions[kFPTraitForULP[p.trait]];
const minusNULP = kMinusNULPFunctions[kFPTraitForULP[p.trait]];or
const trait = kFPTraitForULP[p.trait];
const constants = FP[trait].constants();
const ULPValue = kULPErrorValue[trait];
const plusOneULP = kPlusOneULPFunctions[trait];
const plusNULP = kPlusNULPFunctions[trait];
const minusOneULP = kMinusOneULPFunctions[trait];
const minusNULP = kMinusNULPFunctions[trait];
| const constants = this.constants(); | ||
| const domain_x = [this.toInterval([constants.negative.min, constants.positive.max])]; | ||
| const domain_y = | ||
| this.kind === 'f32' | ||
| this.kind === 'f32' || this.kind === 'abstract' |
There was a problem hiding this comment.
Yes. Thank you for reading this very closely.
The thinking is that we don't want WGSL to specify anything more strict than ECMAScript, and ECMAScript does not specify much in terms of ULP. On the other hand, we also don't want AbstractFloat to be worse than f32.
src/webgpu/util/math.ts
Outdated
| m.every(c => c.length === r), | ||
| `Unexpectedly received jagged array to map` | ||
| ); | ||
| for (let i = 0; i < c; i++) { |
|
Previews, as seen when this build job started (13b3ec7): |
Adds in forwarding of ULP and division interval calls to f32 for abstract
Issue #1626
Requirements for PR author:
.unimplemented()./** documented */and new helper files are found inhelper_index.txt.Requirements for reviewer sign-off:
When landing this PR, be sure to make any necessary issue status updates.