Skip to content

Remove isInf, isNan#2311

Merged
dneto0 merged 1 commit into
gpuweb:mainfrom
dneto0:issue-2270
Nov 18, 2021
Merged

Remove isInf, isNan#2311
dneto0 merged 1 commit into
gpuweb:mainfrom
dneto0:issue-2270

Conversation

@dneto0

@dneto0 dneto0 commented Nov 16, 2021

Copy link
Copy Markdown
Contributor

Fixes: #2270

dneto0 added a commit to dneto0/cts that referenced this pull request Nov 16, 2021
@github-actions

Copy link
Copy Markdown
Contributor

Previews, as seen when this build job started (6b5d2a1):
WebGPU | IDL
WGSL
Explainer

Comment thread wgsl/index.bs
</thead>
<tr algorithm="isNan">
<td rowspan=4>
<tr algorithm="isFinite">

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Shouldn't we remove isFinite too?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Discussed in #2309.

We have agreement on isNan and isFinite. I'd rather land this and then do a second MR for isFinite.

@dneto0 dneto0 merged commit 871df53 into gpuweb:main Nov 18, 2021
github-actions Bot added a commit that referenced this pull request Nov 18, 2021
SHA: 871df53
Reason: push, by @dneto0

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
github-actions Bot added a commit that referenced this pull request Nov 18, 2021
SHA: 871df53
Reason: push, by @dneto0

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
github-actions Bot added a commit that referenced this pull request Nov 18, 2021
SHA: 871df53
Reason: push, by @dneto0

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
dneto0 added a commit to gpuweb/cts that referenced this pull request Nov 18, 2021
shaoboyan added a commit to shaoboyan/tfjs that referenced this pull request Jan 25, 2022
Wgsl removes inNaN from spec (gpuweb/gpuweb#2311)
This CL implement isnan based on the rules in IEEE 754-1985
huningxin pushed a commit to huningxin/dawn that referenced this pull request Feb 7, 2022
Wgsl removes inNaN from spec (gpuweb/gpuweb#2311)
and VertexFormatTest needs to cover NaN input. So we implement a
platform-independent isNaNCustom based on the rules in IEEE 754-1985.

Bug: dawn:1268
Change-Id: I53aef428c72d34381efc6b3ba0250685fc685965
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/78140
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Shaobo Yan <shaobo.yan@intel.com>
qjia7 pushed a commit to tensorflow/tfjs that referenced this pull request Feb 18, 2022
Wgsl removes inNaN from spec (gpuweb/gpuweb#2311)
This CL implement isnan based on the rules in IEEE 754-1985
ben-clayton added a commit to ben-clayton/cts that referenced this pull request Mar 7, 2022
ben-clayton added a commit to gpuweb/cts that referenced this pull request Mar 7, 2022
@almarklein

almarklein commented Mar 1, 2024

Copy link
Copy Markdown
Contributor

In one of our shaders (in pygfx) there can be nans in the input data (storage buffer). We need to detect these nans and make the shader behave differently if a nan is found. Right now this works (code here):

    let is_nan = min(w, 1.0) == 1.0 && max(w, -1.0) == -1.0

We have three questions related to this:

  1. what would be the recommended way to check for nans in input data (no math applied yet).

  2. what happens with nans in the input data in a fastmath implementation? Would it result in bogus data?

  3. I read that on Metal fastmath is on by default. Is this also the case with wgpu/naga?

PS: our use-case is rendering lines, and allowing users to insert gaps by using nans, which is a common trick in dataviz.

@ben-clayton

Copy link
Copy Markdown
Contributor

@almarklein

  1. what would be the recommended way to check for nans in input data (no math applied yet).

WGSL currently makes no guarantees that non-finite f32s can be used in any form. So, even the process of loading a non-finite f32 could result in an undefined value, even if you were to immediately bitcast the result to a u32 and checked the bits.
While intrusive in your implementation, you could replace the use of f32 in the storage buffer with u32, load that value, check for inf / nan bitpatterns, and then if finite, bitcast that to f32.

  1. what happens with nans in the input data in a fastmath implementation? Would it result in bogus data?

It's not just fastmath. For backends that do not explicitly state they support non-finites, you can get undefined values, and we've also seen cases where the backend shader compilers have done some very inexplicable things with flow control. We will likely offer a future extension to support non-finites and expose it for devices that have been tested and shown to work correctly.

  1. I read that on Metal fastmath is on by default. Is this also the case with wgpu/naga?

I can't answer that, I'm afraid.

@almarklein

Copy link
Copy Markdown
Contributor

Thanks for the answers @ben-clayton!

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.

Is it okay that isInf() and isNan() can return false for infinities / nans?

4 participants