Remove/rename `aesara.tensor.nnet` sub-package

[twiecki]

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.

[ricardoV94]

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).

[brandonwillard]

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.

[twiecki]

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 https://github.com/aesara-devs/aesara/issues/674#issuecomment-978605263, 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.

[twiecki]

For someone motivated, they could build a NN package on top of aesara, and this might be a good starting point.

[canyon289]

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.

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?

[rlouf]

I agree we should remove all the functionalities in nnet that are specific to neural nets in Aesara (and signal?).

When I have time I'd like to move these functionalities to a Keras-like library that implements the layers as OpFromGraphs to demonstrate what Aesara can do.

[rth]

So from which module should one import for instance the convolution operator once that sub-package is indeed removed?

[brandonwillard]

So from which module should one import for instance the convolution operator once that sub-package is indeed removed?

We have discussed moving the nnet material to another package, but, since conv is a SciPy-like feature, we could just as well keep it with the existing SciPy support in Aesara. We won't remove it until that next step is clear.

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.