Closed francescopapaleo closed 9 months ago
@bogdanteleaga and @christhetree : what do you think about this?
Thank you Francesco!
I have a couple of thoughts on this:
The PaddingCached and Conv1dCached modules that you have in your example file look identical to the ones we currently have in examples/tcn_1d.py
. We don't want them duplicated and it'd be better to place them inside the neutone_sdk
module like the TODO comments say (perhaps inside this new conv1dcausal file you're creating, maybe rename it to conv1d?). Seems like they only work for mono inputs so they should be generalized as well. For this PR you could just import them from that file and leave this for another time.
Besides that, probably out of scope here, but I think we should move the tcn_1d, gcn_1d, conv1d files inside another module like neutone_sdk.blocks
or neutone_sdk.nn
or something similar.
On a general note, I'd rather not add examples that people cannot run without an existing model file. RAVE is a bit of an exception since it's a popular model and it is helpful for users. But otherwise our other examples are standalone, so overall I think it'd be better if you keep the examples/example_spring_reverb.py file inside the parent repo (https://github.com/francescopapaleo/neutone-spring-reverb) similar to these ones:
And then we place links to them in the README for people to refer to.
Instead we could have the example simply wrap a randomly initialized GCN similar to the TCN example.
Just left two quick comments, one about adding type hints to every parameter everywhere. Otherwise looks great thanks for the work!
LGTM, can you add a link in the README that points to your notebook for the GCN?
Type hints should be ok now and I've added a link to the demo notebook in the README.md file. Thank you for your help!
Thank you! Looks good to me, should we merge @christhetree ?
LGTM
Adding a base class for the GCN network with an example on how to wrap a pre-trained model