-
Notifications
You must be signed in to change notification settings - Fork 133
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here (e.g. What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
By the way, I added test code but couldn't test in my environment. (but tried equivalent code on colab.)
(It's odd message, since I'm not danielzheng, of course) I want someone to test my code before merge. |
@googlebot I signed it! |
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
Thanks for catching this @t-ae! |
@t-ae some tests fail with this error:
Could you please update them appropriately? |
@eaplatanios It's you who added this test. How did you get these expected values? I suspect these values were got from this code itself. |
As I wrote above, I can't build in my environment currently. I'll try that at first. |
OK, I managed to build. I'm not sure what is the main purpose of this test. Anyway, it looks the result values are not what count. |
Yes a lot of the tests are not super well organized/justified right now. Usually we try to make them match some existing working implementation (e.g., using Python). This looks good to me now. Thanks for updating it! |
I tried the code below on colab.
It looks
dsigmoid
returns wrong value.sigmoid
and its gradient is defined here.swift-apis/Sources/TensorFlow/Operators/Math.swift
Lines 962 to 975 in 323b564
And
Raw.sigmoidGrad
is here.https://raw.githubusercontent.com/tensorflow/swift-apis/323b5640c48822340f2f5410d6bff51972fc9f45/Sources/TensorFlow/Bindings/RawOpsGenerated.swift
_vjpSigmoid
passesx
but whatsigmoidGrad
requires isy
.