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

A little bug! #48

Closed
real-zhangzhe opened this issue Apr 8, 2020 · 8 comments
Closed

A little bug! #48

real-zhangzhe opened this issue Apr 8, 2020 · 8 comments
Labels
bug Something isn't working

Comments

@real-zhangzhe
Copy link

Hi, I think there is a little bug at numpy-ml/numpy_ml/neural_nets/activations/activations.py Line 64.

your code

fn_x = self.fn_x

but

self.fn_x Never defined

@real-zhangzhe
Copy link
Author

And numpy-ml/numpy_ml/neural_nets/wrappers/wrappers.py Line 207:

your code

def backward(self, dLdy, retain_grads):
"""
retain_grads: Default is True
"""

Your code is missing the default value, resulting in an error at numpy-ml/numpy_ml/neural_nets/layers/layers.py Line 332.

@real-zhangzhe
Copy link
Author

And numpy-ml/numpy_ml/neural_nets/layers/layers.py Line 2116:

your code

def backward(self, dLdy):
"""
retain_grads: Default is True
"""

The function in your code is missing an argument retain_grads, resulting in an error at numpy-ml/numpy_ml/neural_nets/wrappers/wrappers.py Line 227.

@real-zhangzhe
Copy link
Author

Maybe I should fork and create pull request 😃

@real-zhangzhe
Copy link
Author

And numpy-ml/numpy_ml/neural_nets/tests/tests.py line 510:

your code

from ..activations import Softmax

but

Softmax is not implemented in ..activations, but in ..layers.

right code

  • line 510:
from ..layers import Softmax

  • line 527:
y_pred = sm.forward(z)

@real-zhangzhe
Copy link
Author

real-zhangzhe commented Apr 9, 2020

And numpy-ml/numpy_ml/neural_nets/tests/tests.py line 771:
your code

from ..activations import SoftSign

but SoftSign is not implemented in ..activations.

maybe you should delete function test_softsign_grad and test_softsign_activation.

@ddbourgin
Copy link
Owner

@Z-zhe - Wow, thanks so much for all these! I haven't had a chance to take a look yet, but should have some time this weekend. In the meantime if you feel like submitting a PR with fixes I'd be happy to review it, otherwise I can try to address these shortly.

@ddbourgin ddbourgin added the bug Something isn't working label Apr 9, 2020
@real-zhangzhe
Copy link
Author

PR is complicated, it is easier for you to modify. 😃

@real-zhangzhe
Copy link
Author

numpy-ml/numpy_ml/neural_nets/layers/layers.py line 2341:

your code

dX = dZ @ W.T

I don't think it should be W it should be W_sparse. So I think right code should be:

dX = dZ @ W_sparse.T

Please reconsider,thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants