Closed thargy closed 6 years ago
Hi @mellinoe, is there any more info you need for this?
@thargy Sorry, this has been on my list. I'll try to get to it later today or tomorrow.
No problems, just wasn't sure if you'd clocked it. I've merged the latest master changes you pushed.
There's a bit more complexity here than I'd like, but hopefully that will pay off in the future as we work with the codebase more. If you fix the compute shader name (left a comment about it), then all the tests pass for me on my systems. There's a few other misc. things that should be fixed, but overall it looks to be in an okay state.
There's a bit more complexity here than I'd like, but hopefully that will pay off in the future as we work with the codebase more. If you fix the compute shader name (left a comment about it), then all the tests pass for me on my systems. There's a few other misc. things that should be fixed, but overall it looks to be in an okay state.
Though it might appear a little complicated I've tried to make it encapsulated and it can be easily used to test other methods, and, eventually, extended to test GPU-only methods (comparing between backends only and not CPU).
I think it'll pay off as it has taken me about an hour to correctly identify and fix inconsistencies between the various backends, you can see the changes in f9a7b579aac7753e4e5ce9f1c1e4b319125cfbbe. The Builtins are now consistent across all backends (and CPU). As it is now testing 1000 random iterations for each of the 165 methods that are implemented on GPU & CPU across all backends, writing manual tests would have been significantly more time consuming and prone to missing things. Only Atanh is currently showing problems on Direct3D.
@mellinoe I believe I've implemented all your suggestions, and with the fix of Atanh
I've just pushed (4f40651cf94748ab252ed289132a63e917273dac), then the builtins are all showing as equivalent across all backends (albeit for relatively constrained input values). When you start loosening the constraints on the input floats then the Trig functions start to show inconsistencies (unsurprisingly).
I haven't, however, been able to test on metal, so you'll need to look at the TestBuiltins
output to see if Metal shows any inconsistencies against the other backends. (Comparing against just the CPU is a decent start as this has been validated as being close enough to the GPU implemenations already).
There's a couple of problems on Metal with the latest changes.
int index = (int)ShaderGen.ShaderBuiltins.DispatchThreadID.X;
The cast (UInt3->int) fails on Metal otherwise.
I'm also hitting a crash when the tests complete on Windows:
The active test run was aborted. Reason: Unhandled Exception: Unhandled Exception: System.ObjectDisposedException: Safe handle has been closed
at System.Runtime.InteropServices.SafeHandle.DangerousAddRef(Boolean& success)
at System.StubHelpers.StubHelpers.SafeHandleAddRef(SafeHandle pHandle, Boolean& success)
at Microsoft.Win32.Win32Native.SetEvent(SafeWaitHandle handle)
at System.Threading.EventWaitHandle.Set()
at ShaderGen.Tests.Tools.ToolChain.<>c__DisplayClass44_1.<Execute>b__0(Object sender, DataReceivedEventArgs e) in E:\projects\ShaderGen\src\ShaderGen.Tests\Tools\ToolChain.cs:line 536
Here's what my fix for AddCheck looks like:
private static readonly string[] _vectorAccessors = { "0", "1", "2", "3" };
...
return
$"{shaderType}({string.Join(",", _vectorAccessors.Take(elementCount).Select(a => check.Replace("`", "[" + a + "]")))})";
Here's what my fix for AddCheck looks like:
I already pushed:
return
$"{shaderType}({string.Join(",", Enumerable.Range(0, elementCount).Select(a => check.Replace("`", $"[{a}]")))})";
Which should be equivalent (and dropped _vectorAccessors).
I'm also hitting a crash when the tests complete on Windows:
I've never seen that myself, however reviewing the code suggests a possibility. On exit, if one of the tools is still running the test process will start shutting down and disposing objects. The outputWaitHandle
will be disposed, as will the errorWaitHandle
. If the child process is now terminated/finishes, it may return data, and the OutputDataReceived
event will fire, and you would see the error shown.
I've pushed a change that will make sure the process is disposed prior to the waithandles being disposed. Hopefully that will, at the very least, de-register the event handlers and prevent the error you're seeing be posisble.
I ran it a few times and, while it seems to occur less frequently, it still crashes occasionally (2/6 times currently).
Metal seems to be good now -- no problems there any more.
I ran it a few times and, while it seems to occur less frequently, it still crashes occasionally (2/6 times currently).
What test runner are you using on Windows, I'm just running in VS using the R# runner?
Metal seems to be good now -- no problems there any more.
Are there any failures reported in TestBuiltins
(you'll still get a tick). If there are no issues the output should end:
Analysing results for failures took 1470.02ms
No failures detected!
I'm just running dotnet test ShaderGen.Tests.csproj
from the command line.
@thargy Ah, I didn't realize there could be failures while the test still passed. Here's what it outputs:
Analysing results for failures took 824.3ms 7 methods had failures out of 165 (4.24%).
● Vector4 ShaderGen.ShaderBuiltins.Mod(Vector4 a, Vector4 b) failed 950/1000 (95%).
Mod(<-0.1473419, -0.06606695, 3.790649, -3.812556>, <-0.5484863, 1.955159, 0.2919904, -0.2313264>) CPU = <-0.1473419, 1.889092, 0.2867646, -0.1113338> Metal = <-0.1473419, -0.06606695, 0.2867646, -0.1113338>
Mod(<-3.672568, 3.733981, -3.982012, 0.04768488>, <0.08522116, -0.6115583, 2.423151, -0.1089887>) CPU = <0.0771637, -0.5469265, 0.8642893, -0.06130387> Metal = <-0.008057557, 0.06463158, -1.558861, 0.04768488>
Mod(<-3.337058, 3.664641, -0.04931281, -0.2034206>, <0.1302528, 0.3109234, -1.64931, -0.7285025>) CPU = <0.04951525, 0.2444837, -0.04931281, -0.2034206> Metal = <-0.08073749, 0.2444837, -0.04931281, -0.2034206>
Mod(<0.06146766, 0.2269077, -0.08715918, 0.7744274>, <-0.1101076, 0.1933157, -0.08376742, -1.092888>) CPU = <-0.0486399, 0.033592, -0.003391758, -0.3184605> Metal = <0.06146766, 0.033592, -0.003391758, 0.7744274>
Mod(<0.2180625, -0.6099011, -0.1890869, -0.1876781>, <0.09211984, -0.14133, 0.2206808, 0.320484>) CPU = <0.03382286, -0.04458094, 0.03159387, 0.1328059> Metal = <0.03382286, -0.04458094, -0.1890869, -0.1876781>
…
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
● Vector4 ShaderGen.ShaderBuiltins.Mod(Vector4 a, Single b) failed 936/1000 (93.6%).
Mod(<-1.332414, -0.1519372, 0.3580664, 0.2030616>, -0.2266406) CPU = <-0.199211, -0.1519372, -0.09521487, -0.02357905> Metal = <-0.1992111, -0.1519372, 0.1314258, 0.2030616>
Mod(<-0.2432121, -0.4899388, -0.1968114, 1.390035>, 0.7634823) CPU = <0.5202702, 0.2735436, 0.566671, 0.6265526> Metal = <-0.2432121, -0.4899388, -0.1968114, 0.6265526>
Mod(<1.875447, 0.08230057, 0.4035302, -0.04449578>, -1.059721) CPU = <-0.2439942, -0.97742, -0.6561903, -0.04449578> Metal = <0.8157263, 0.08230057, 0.4035302, -0.04449578>
Mod(<-0.9921335, 0.2405954, 0.02952472, -0.1325384>, -0.3359732) CPU = <-0.320187, -0.09537782, -0.3064485, -0.1325384> Metal = <-0.320187, 0.2405954, 0.02952472, -0.1325384>
Mod(<0.7359577, -1.616542, 0.4873287, -0.447501>, 0.6781166) CPU = <0.05784118, 0.4178079, 0.4873287, 0.2306156> Metal = <0.05784118, -0.2603086, 0.4873287, -0.447501>
…
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
● Vector3 ShaderGen.ShaderBuiltins.Mod(Vector3 a, Vector3 b) failed 887/1000 (88.7%).
Mod(<-0.06014774, -0.9836, 0.02765148>, <0.1230681, 0.8742101, -0.1192536>) CPU = <0.06292035, 0.7648202, -0.09160215> Metal = <-0.06014774, -0.1093899, 0.02765148>
Mod(<0.0284753, -0.1399699, 0.07027472>, <-0.6422678, -0.2835048, 0.1584362>) CPU = <-0.6137925, -0.1399699, 0.07027472> Metal = <0.0284753, -0.1399699, 0.07027472>
Mod(<3.830129, -0.2082705, -0.3100274>, <-0.7649199, 3.870798, -0.344797>) CPU = <-0.7593904, 3.662527, -0.3100274> Metal = <0.005529761, -0.2082705, -0.3100274>
Mod(<0.1018435, -0.1338812, 0.1493706>, <0.1147475, 3.891926, -1.851492>) CPU = <0.1018435, 3.758044, -1.702121> Metal = <0.1018435, -0.1338812, 0.1493706>
Mod(<-0.1772397, 0.1387893, -0.1407941>, <-0.8813902, 0.6458945, 0.04376643>) CPU = <-0.1772397, 0.1387893, 0.03427166> Metal = <-0.1772397, 0.1387893, -0.00949477>
…
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
◕ Vector3 ShaderGen.ShaderBuiltins.Mod(Vector3 a, Single b) failed 868/1000 (86.8%).
Mod(<-1.485478, 1.124717, -0.3238766>, -0.259662) CPU = <-0.1871682, -0.1735935, -0.06421456> Metal = <-0.1871683, 0.08606851, -0.06421456>
Mod(<-0.05152696, 0.1360926, 0.1211376>, 0.1381756) CPU = <0.08664868, 0.1360926, 0.1211376> Metal = <-0.05152696, 0.1360926, 0.1211376>
Mod(<0.02352959, -0.5231687, 1.2423>, -0.1055625) CPU = <-0.08203294, -0.1009186, -0.02445054> Metal = <0.02352959, -0.1009186, 0.08111196>
Mod(<1.713617, 1.400487, -0.1511525>, 0.3490644) CPU = <0.3173599, 0.004229307, 0.1979119> Metal = <0.3173599, 0.004229307, -0.1511525>
Mod(<1.736485, -0.2102596, -1.810122>, -1.510112) CPU = <-1.283738, -0.2102596, -0.30001> Metal = <0.2263739, -0.2102596, -0.30001>
…
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
◕ Vector2 ShaderGen.ShaderBuiltins.Mod(Vector2 a, Vector2 b) failed 757/1000 (75.7%).
Mod(<-0.06524262, 0.1453021>, <0.1248452, -0.1563947>) CPU = <0.05960259, -0.01109263> Metal = <-0.06524262, 0.1453021>
Mod(<-0.24629, -0.1658617>, <-0.1526749, 0.08190227>) CPU = <-0.09361501, 0.07984507> Metal = <-0.09361501, -0.002057195>
Mod(<0.1217519, -0.08054987>, <-0.06699717, 0.1960435>) CPU = <-0.01224243, 0.1154936> Metal = <0.05475474, -0.08054987>
Mod(<-0.02917885, 0.3808856>, <0.423089, -0.2923595>) CPU = <0.3939101, -0.2038334> Metal = <-0.02917885, 0.0885261>
Mod(<-1.068448, 0.1429756>, <0.2403389, -0.2420461>) CPU = <0.1332461, -0.09907053> Metal = <-0.1070929, 0.1429756>
…
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
◕ Vector2 ShaderGen.ShaderBuiltins.Mod(Vector2 a, Single b) failed 747/1000 (74.7%).
Mod(<-0.4987893, 2.1514>, 0.3335225) CPU = <0.1682557, 0.150265> Metal = <-0.1652668, 0.150265>
Mod(<-0.04075132, 0.1077888>, -0.8046364) CPU = <-0.04075132, -0.6968477> Metal = <-0.04075132, 0.1077888>
Mod(<3.513057, -0.3537728>, 0.4259948) CPU = <0.1050982, 0.07222205> Metal = <0.1050982, -0.3537728>
Mod(<0.09067725, -1.519614>, 0.9782577) CPU = <0.09067725, 0.4369016> Metal = <0.09067725, -0.5413561>
Mod(<0.3394809, -0.05812054>, 0.1904949) CPU = <0.148986, 0.1323744> Metal = <0.148986, -0.05812054>
…
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
◑ Single ShaderGen.ShaderBuiltins.Mod(Single a, Single b) failed 509/1000 (50.9%).
Mod(0.523179, -0.3522496) CPU = -0.1813202 Metal = 0.1709294
Mod(-0.8329723, 0.1716798) CPU = 0.02542657 Metal = -0.1462532
Mod(-0.71939, 0.09926502) CPU = 0.07473016 Metal = -0.02453486
Mod(-0.9161512, 0.2987621) CPU = 0.278897 Metal = -0.01986507
Mod(-0.2505285, 0.4783684) CPU = 0.2278398 Metal = -0.2505285
…
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
I'm just running dotnet test ShaderGen.Tests.csproj from the command line.
If run this repeatedly now without any failures locally :(.
As for metal, can you try changing line 55 in MetalKnownFunctions
:
{nameof(ShaderBuiltins.Mod), SimpleNameTranslator("fmod")},
to
{nameof(ShaderBuiltins.Mod), SimpleNameTranslator()},
in the first instance?
Plain old mod
doesn't seem to be a standard Metal function:
/var/folders/8t/vt43x2517k569rdz9_wbv5dr0000gn/T/tmp5Q37rV.tmp:502:23: error: use of undeclared identifier 'mod'
parameters.Single_3 = mod(parameters.Single_1, parameters.Single_2);
^
Plain old mod doesn't seem to be a standard Metal function:
Apparently, there is a '%' operator that might be equivalent so I've pushed some code for that, if not, it will be similar to the FMod
function except it should use floor
in place of trunc
.
If run this repeatedly now without any failures locally :(.
Finally got a failure! I've pushed a commit with try...catch around the sets for the rare race condition.
Metal's %
operator doesn't seem to work with floats or vectors:
Error:
/var/folders/8t/vt43x2517k569rdz9_wbv5dr0000gn/T/tmpuMIHxt.tmp:87:10: error: invalid operands to binary expression ('float' and 'float')
r = f%2;
~^~
/var/folders/8t/vt43x2517k569rdz9_wbv5dr0000gn/T/tmpuMIHxt.tmp:88:23: error: invalid operands to binary expression ('float2' (aka 'vector_float2') and 'float2')
r2 = float2(VH.V2)%float2(float2(2, 4));
~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~
/var/folders/8t/vt43x2517k569rdz9_wbv5dr0000gn/T/tmpuMIHxt.tmp:89:23: error: invalid operands to binary expression ('float3' (aka 'vector_float3') and 'float3')
r3 = float3(VH.V3)%float3(float3(2, 4, 6));
~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~
/var/folders/8t/vt43x2517k569rdz9_wbv5dr0000gn/T/tmpuMIHxt.tmp:90:23: error: invalid operands to binary expression ('float4' (aka 'vector_float4') and 'float4')
r4 = float4(VH.V4)%float4(float4(2, 4, 6, 8));
~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
4 errors generated.
it will be similar to the FMod function except it should use floor in place of trunc.
That seems to work well enough.
Analysing results for failures took 724.48ms
1 methods had failures out of 165 (.61%).
○ Vector2 ShaderGen.ShaderBuiltins.Mod(Vector2 a, Single b) failed 1/1000 (.1%).
Mod(<-0.2273049, -2.378017>, -3.592751E-06)
CPU = <-2.324581E-06, 0>
Metal = <-2.32918E-06, 8.981442E-08>
@thargy I merged this manually and applied the suggested implementation for Metal's Mod
function. Feel free to open up another PR if there's further changes that should be made.
Thanks for the contribution!
This pull request replaces the manual ShaderBuiltinsTest with a new auto-generated code test suite. The new test (
TestBuiltins
) accepts a list of methods that can run on both CPU & GPU and then builds the code for each available backend automatically. It then generates random test data and runs multiple iterations against the methods under test, storing the results. Finally, it analyses the differences between backends, and the CPU and produces a report similar to:Which clearly shows an issue in the way the CPU code for
SmoothStep
is implemented.Or:
From which it much easier to start diagnosing where issues are occurring, particularly as it shows the values that resulted in failures occurring.