Mixed fixes

[abergeron]

Some accumulated small fixes. Most notably float16 is changed to a struct to avoid bad results.

[nouiz]

version bump? Do Theano tests still pass?

[abergeron]

I don't know about Theano, we would have to try.

[abergeron]

The latest commits will require some changes to Theano (mainly to remove the atomicAdd/Exch implementation in GpuAdvancedIncSubtensor1_dev20). These functions cheat a bit so they are affected by the structure change.

The tests with Theano are running so we'll see what else falls out.

[abergeron]

There are 25 test failures in Theano. Two of those seem to have nothing to do with float16 but the others look like fallout from the struct change.

Given that, I'm not willing to make the change in a minor release. I would rather push this back to 0.7. The other fixes in the PR should be fine though. I can isolate them.

[nouiz]

Ok to move the float16 struct change to another PR and 0.7 But I think we should do that soon. Keep the failed test in Theano with that changes. They are indication of places that could give wrong result. Can you look at them to see if you can spot such stuff?

…

On Thu, May 25, 2017 at 8:14 AM abergeron ***@***.***> wrote: There are 25 test failures in Theano. Two of those seem to have nothing to do with float16 but the others look like fallout from the struct change. Given that, I'm not willing to make the change in a minor release. I would rather push this back to 0.7. The other fixes in the PR should be fine though. I can isolate them. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#442 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AALC-8hYWGMAKAUlUtEMmlTvMDA1QRfnks5r9XCxgaJpZM4NkCLV> .

[abergeron]

I've removed the float16 commits from this. I'll check the failed tests in Theano for wrong results.

[abergeron]

It seems that all of the failures in Theano come from something like this:

kernel void f(ga_half *res) {
  res[0] = __float2half_rn(value);
}

And the error is something like Can't store 'unsigned short' in 'struct _ga_half'

So there doesn't appear to be any wrong-results stuff hiding there, but we might want to flesh out the struct a bit to avoid having to do those conversions manually. I'll leave that for later.