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

Float literal is infinite #5515

Closed
apessino opened this issue Apr 10, 2024 · 12 comments
Closed

Float literal is infinite #5515

apessino opened this issue Apr 10, 2024 · 12 comments
Labels
type: question Further information is requested

Comments

@apessino
Copy link

After the latest sync, this error showed up in my shaders wherever I do a division by 0.0:

Shader 'surface.wgsl' parsing error: Float literal is infinite
    ┌─ wgsl:348:27
    │
348 │         v.pos = vec4<f32>(1.0f / 0.0f); // send to infinity
    │                           ^^^^^^^^^^^ see msg

I don't understand the logic behind this - these are floating point numbers, infinities are well defined and necessary. A division by zero is the most neutral way to obtain an INF, and this has always worked. A small issue but, IEEE754 notwithstanding, I would rather not slap a constant in there, especially since I am not aware of predefined, common constants in wgsl.

Is there a particular reason why this error was introduced? Thanks...

@Wumpf Wumpf added the type: question Further information is requested label Apr 10, 2024
@Wumpf
Copy link
Member

Wumpf commented Apr 10, 2024

infinities are well defined

I with this was true, but WebGPU decided to leave infinities undefined since apparently too many implementations don't handle them correctly. Going one step further any constant that evaluates to infinity is required to report an error and Naga/wgpu does this now correctly

The details are documented here https://www.w3.org/TR/WGSL/#floating-point-evaluation

Let X be an infinite-precision intermediate result from a floating point computation.
...
If X' is infinity or NaN, then:
* If the expression is a const-expression, generate a shader-creation error.
...

@Wumpf Wumpf closed this as not planned Won't fix, can't repro, duplicate, stale Apr 10, 2024
@Wumpf
Copy link
Member

Wumpf commented Apr 10, 2024

There's still an open issue here whether to add an extension to properly define infinities:

@apessino
Copy link
Author

Thanks for clarifying! I can't say that I agree with any of it, but I appreciate you taking the time to fill me in as to the reasoning.

Cheers!

A=

@Wumpf
Copy link
Member

Wumpf commented Apr 10, 2024

Got curious myself where this came from exactly, here's the ticket that details the survey behind the decision:

@apessino
Copy link
Author

Those IEEE rules are ancient, and the exceptions have not been enabled by default since the early '90s FPUs. They are useless other than in insanely specific cases.

In practice, we have come to expect and even rely on NaNs and infinities for all kinds of things - I find it hilarious that rather than just ignoring them as everything else does, they decided to enforce them with no option to bypass. In shader code, at that, where they are even less relevant. And, of course, they will still be ignored when they occur in runtime, because it makes sense for them to be ignored.

Oh well... ;)

A=

@Wumpf
Copy link
Member

Wumpf commented Apr 10, 2024

maybe worth opening an issue on WebGPU and discussing it there some more. I got hit badly by this as well when relying on NaN and infs myself :/

@apessino
Copy link
Author

I'd be all for joining in, if you could use some moral support.

I like wgpu very much, I believe it is an important step in the right direction and I am forcing myself to stick with wgsl, but there is an odd pedantic formalism around the whole thing that's dragging back progress.

Reading the wgsl spec document is a tragicomical experience, in my opinion. It's a simple shading language, in no way a departure or a leap over dozens of existing other offerings, and yet the spec reads like it's Andrew Wiles' proof of Fermat's Last Theorem. ;)

The language itself has gotten better, but it's still obtuse in arbitrary ways that make practical use more cumbersome and verbose than it has any reason to be. I have dreams about the day I can stop putting a pointless "u" next to each damn unsigned integer literal I write.

Anyhow... sorry for venting, and once again I appreciate your quick reply!

@apessino
Copy link
Author

In the meantime, this works:

fn ratio(n: f32, d: f32) -> f32 {
    return n / d;
}
fn inf() -> f32 {
    return ratio(1.0, 0.0);
}

// ...
v.pos = vec4<f32>(inf());

😂

@cwfitzgerald
Copy link
Member

cwfitzgerald commented Apr 10, 2024

Welcome to the fun that is web specifications, and realizing just how varying graphics API specs are!

I have dreams about the day I can stop putting a pointless "u" next to each damn unsigned integer literal I write.

I think this works per the spec - the type of a literal is AbstractInt which should be coerced into whatever makes the most sense in context. I'm not sure how thorough our implementation is in 0.19 or trunk.

The language itself has gotten better, but it's still obtuse in arbitrary ways that make practical use more cumbersome and verbose than it has any reason to be.

Yeah we feel this too, it's getting better as more sugar and utilities are being added post-v1. We're very far from the general usefullness of HLSL, but getting closer to where GLSL is.

Reading the wgsl spec document is a tragicomical experience, in my opinion. It's a simple shading language, in no way a departure or a leap over dozens of existing other offerings, and yet the spec reads like it's Andrew Wiles' proof of Fermat's Last Theorem. ;)

I definitely understand the sentiment :) This just seems to be how specifications need to be. Vulkan/Spirv specs are similarly comically large for their scope, and I still sometimes curse things that are underspeced.

I think in this case it's especially important to be really thorough in the spec for WGSL, as GLSL and HLSL aren't well speced, and there's constant questions about what is and isn't allowed, and how the behavior is supposed to happen. Having a very strict spec, allows implementers to know if they're behaving correctly or not, reducing variance between implementations (a historical issue for WebGL).

I like wgpu very much, I believe it is an important step in the right direction and I am forcing myself to stick with wgsl, but there is an odd pedantic formalism around the whole thing that's dragging back progress.

Design by committee specification: a necessary evil that runs much slower than everyone wants it to. This is why wgpu itself has native only features, so we can move faster than the spec can and implement things before the spec.

In the meantime, this works:

fwiw, this will probably work, but the behavior of this isn't guaranteed in any way, I think by any underlying spec. Inf and NaN are assumed never to happen. Ain't it fun!

@apessino
Copy link
Author

Thanks for the reply, and for indulging my annoying venting. I swear I don't do that normally, and I certainly don't mean to be unappreciative of the work and exceptional effort your team puts into this work - but I think I am one of the heaviest users (and earliest adopters) and sometimes that comes with a few frustrations.

I think this works per the spec - the type of a literal is AbstractInt which should be coerced into whatever makes the most sense in context. I'm not sure how thorough our implementation is in 0.19 or trunk.

It works in a few spots (simple integer expressions, for example) but call a function that takes a u32 and pass a 0 and you get this beauty:

Shader validation error:
    ┌─ surface.wgsl:367:12
    │
367 │     return with_cond(vertex_ix, 0);
    │            ^^^^^^^^^^^^^^^^^^^^^^^
    │            │                    │
    │            │                    naga::Expression [2]
    │            invalid function call


    Entry point vs_main_cond at Vertex is invalid
    Call to [11] is invalid
    Argument 1 value [2] doesn't match the type [1]

It wants a 0u. Don't get me started on that error message... my goodness.

I definitely understand the sentiment :) This just seems to be how specifications need to be. Vulkan/Spirv specs are similarly comically large for their scope, and I still sometimes curse things that are underspeced.

What I find heavy is not the size. You can stuff all the detail you can imagine in there, as long as the data is meaningful. It's the masturbatory verbosity and the adherence to this imagined pseudo-formalism that seems like comedy. It begins by defining the meaning of "is" and "must" for crying out loud.

Anyhow, I get it... it's what design by committee will get you. We must take the good and the strange with stoic resilience. 😘

Thanks again!

A=

@jimblandy
Copy link
Member

jimblandy commented Apr 11, 2024

@apessino As a contributor to the WGSL spec, I wanted to speak up in its defense a bit:

It begins by defining the meaning of "is" and "must" for crying out loud.

This sort of clarification became common long ago, I assume as a means of drawing a clearer distinction between mere description, and the statement of requirements for conformance. For example:

  • RFC 2119: Key words for use in RFCs to Indicate Requirement Levels

    This is used extensively by other internet RFCs.

  • The Vulkan spec has a similar section: §2.1.3 Normative Terminology

But, say, the C++ standard doesn't have anything like this. So it's not universal or essential. But I have bad news: once you start reading specs a lot, this will be the least of your worries.

Ideally, the WGSL spec would be perfectly legible and yet guide implementors with enough specificity to ensure portability between browsers. But even as it stands today, I'd take the WGSL spec any day over the Metal Shading Language spec or the Direct3D HLSL spec (... where is that? Can you even find me a grammar for HLSL?). The GLSL spec is much better than those, but it still leaves a bit to the imagination, which a spec should not.

If you want to see another spec shaped by the excruciating portability demands that web browsers suffer under, check out the ECMAScript Spec, which I find bewildering to consult because it defines JavaScript almost entirely in pseudocode, with almost no English to provide rationale or hints as to the intent.

As to the original topic of this issue: from my point of view, WGSL just helped you avoid writing non-portable code. "You're welcome!" :)

The unnecessary u suffixes are my fault - I still need to handle some more cases in the WGSL front end. I've filed #5523 for this case.

@jimblandy
Copy link
Member

jimblandy commented Apr 11, 2024

Don't get me started on that error message... my goodness.

This is indeed a point of shame for Naga. The error messages are awful. We're tracking this work under the "kind: diagnostics" label.

Once we can actually handle the shaders people are trying to hand us correctly, we really want to improve the diagnostics.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants