-
-
Notifications
You must be signed in to change notification settings - Fork 156
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
Remove/rename aesara.tensor.nnet
sub-package
#674
Comments
There are a lot of things there that we might not want to move. All those crossentropy / bias variations. We already moved sigmoid and softplus out, and I think the next step is to move softmax/log_softmax out (#673). |
I would go so far as to say that we should disable auto-importing and add a deprecation warning to the Also, since the tests for that sub-package take a good amount of our CI time (~2 hours), we could subsequently disable testing for it and significantly speed up testing. |
That makes sense.
…On Thu, Nov 25, 2021, 01:42 Brandon T. Willard ***@***.***> wrote:
I would go so far as to say that we should disable auto-importing and add
a deprecation warning to the aesara.tensor.nnet sub-package ASAP, because
we don't intend to maintain any of that neural network code as part of core
Aesara.
Also, since the tests for that sub-package take a good amount of our CI
time (~2 hours), we could subsequently disable testing for it and
significantly speed up testing.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#674 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAFETGHCKQQ5F256VZE3373UNWA7ZANCNFSM5IWJN7PQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
For someone motivated, they could build a NN package on top of aesara, and this might be a good starting point. |
aesara.tensor.nnet
sub-package
Agree with this, it was a huge slowdown when I was getting developing with aesara both because of all the things that broke in it when I made changes, and the testing slowdown. Removing would streamline codebase tremendously. I dont think it needs a deprecation warning since aesara is a fork and its not like we promised to maintain its functionality. Can we just immediately delete it? |
I agree we should remove all the functionalities in When I have time I'd like to move these functionalities to a Keras-like library that implements the layers as |
So from which module should one import for instance the convolution operator once that sub-package is indeed removed? |
We have discussed moving the Regardless, if there are people still depending on this code, we will at the very least keep it available in some form before removing it from here. |
I think having
nnet
as a name is pretty confusing, these are general math functions not only used for neural nets, and by now Aesara isn't geared towards neural nets anyway. Maybe we can rename (to what?) or, better I think, just move it down to the regular name space.The text was updated successfully, but these errors were encountered: