-
Notifications
You must be signed in to change notification settings - Fork 25
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
Design for Base Deep class #26
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change requests:
- I think we need a very careful write-up of the conceptual model. Which abstractions/objects are there? Which abstractions are represented by which base classes? What are (formal) examples?
- one related specific point: previously, I think the
BaseDeepNetwork
descendants always contained uncompiled networks? Whereas, in your solution 1, it seems to be a compiled network now. What is the generic signature ofbuild_model
? - I would also follow the "3 examples rule" in design. If you define a base class or strategy pattern, list 3 examples. This also applies to examples of, say, composition.
Hey,
|
hey @fkiraly , |
@fkiraly I have updated the doc to make it more understandable. |
hi @AurumnPegasus great stuff, but I think if we are going for a full design rather than a quick port (which was my preference tbh, sktime-dl is simply ported from https://github.com/hfawaz/dl-4-tsc), we might want to consider pytorch too. I have never used pytorch myself, but it is probably more popular now in my areas of research. pytorch is, I believe, structured quite differently. Do you have any experience with it? |
Hey @TonyBagnall There might be minor differences which will be there in |
it would be awesome to have a structure that allowed both pytorch and keras based estimators. Id like to keep the current implementations as they reproduce Hassan's results, but ideally I'd like researchers to use sktime to design new estimators, and pytorch functionality would I think greatly help that. |
I would suggest, let's not overcomplicate the design question by having multiple backends. We already have multiple estimator types, that is imo the challenge we first should address. |
Hi @AurumnPegasus, thanks for the solutions proposed here, good job 👍 |
hey @GuzalBulatova So in this case BaseDeepEstimator does not really multiply entities from my understanding, although I am not very sure what you mean by entities here |
Hi @AurumnPegasus I see, I got it wrong previous time. I thought you propose BaseDeepNetwork -> BaseDeepEstimator(BaseDeepNetwork, BaseEstimator) -> BaseDeepClassifier(BaseDeepEstimator) -> anydeepclassifier. And in this case I was wondering if it made sense to have a BaseDeepNetwork as a separate class. I think I understand now what you meant. It makes sense, I would go ahead with the second option. |
Any other comments?
I have proposed 2 solutions for the structure of Deep Learning Estimators (personally, I prefer the second).
I would like any suggestions/changes I can make to the designs, and would love to hear feedback.