-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
[ENH] Migrating Estimators from sktime-dl #3351
Comments
quick question about the recipe: do we always need to create a separate network? Or are there a few common ones? E.g., for regression/classification? Should the regressor/classifier not use the same network? |
Yup, made those changes. Will add more instructions as well |
I want to work on the implementation of ResNet Classifier and regressor, but i could not understand the migration steps. @fkiraly @AurumnPegasus can you please help me. |
Hey @nilesh05apr , Firstly, create an issue for the estimator you want to work on. Then, So firstly, go to the network code in sktime-dl. This is the code you need to migrate to sktime. Create a file in To create the Classifier, go to classifier code in sktime-dl. This is the code you need to migrate to sktime. Create a file in You can import the documentation following the way it has been written in either MLPClassifier or CNNClassifier/CNNRegressor. I hope it helps! Let me know if you get stuck somehwere. |
Thanks @AurumnPegasus for the clarification, it's much clear now. I have started working on it. |
@AurumnPegasus @fkiraly i have made a pr will resnet implemented can you please review it and also assign me this issue. i will migrate all the estimators from this sktime_dl to sktime. |
@fkiraly I have updated the wordings and migration instructions, hope they are better. I will add the code and examples for special cases in some time (since we have not really agreed to a couple of cases yet). |
@AurumnPegasus can you please link steps to test the changes locally. |
Hey @nilesh05apr , Once that is done, to test the estimators (lets say you want to test Then, run the Hope it helps! |
I'll work on TapNet Classifier Migration. Do add #3372 to the list. |
Hey, taking up one regressor model as well. Kindly add #3474 to the list. |
Hi there!!! |
Hey @ArushikaBansal, thanks for taking this up. In the specific case of You can use this file to create both the network and the estimator so it shouldn't be much of trouble. In case you face any difficulty, feel free to ask me or any other core dev you want to ask. (Do keep in mind to use one of the already ported estimators as a reference, that will make things easier.) |
Thankyou sir for the help!!!!! |
I made the required changes please review them |
<!-- Thanks for contributing a pull request! Please ensure you have taken a look at our contribution guide: https://github.com/sktime/sktime/blob/main/CONTRIBUTING.md --> #### Reference Issues/PRs <!-- Example: Fixes #1234. See also #3456. Please use keywords (e.g., Fixes) to create link to the issues or pull requests you resolved, so that they will automatically be closed when your pull request is merged. See https://github.com/blog/1506-closing-issues-via-pull-requests --> Fixes #3294 Part of #3351 #### What does this implement/fix? Explain your changes. <!-- A clear and concise description of what you have implemented. --> Add `MACNNNetwork` and `MACNNClassifier`. #### Does your contribution introduce a new dependency? If yes, which one? <!-- If your contribution does add a new hard dependency, we may suggest to initially develop your contribution in a separate companion package in https://github.com/sktime/ to keep external dependencies of the core sktime package to a minimum. --> None #### What should a reviewer concentrate their feedback on? <!-- This section is particularly useful if you have a pull request that is still in development. You can guide the reviews to focus on the parts that are ready for their comments. We suggest using bullets (indicated by * or -) and filled checkboxes [x] here --> #### Did you add any tests for the change? <!-- This section is useful if you have added a test in addition to the existing ones. This will ensure that further changes to these files won't introduce the same kind of bug. It is considered good practice to add tests with newly added code to enforce the fact that the code actually works. This will reduce the chance of introducing logical bugs. --> #### Any other comments? <!-- Please be aware that we are a loose team of volunteers so patience is necessary; assistance handling other issues is very welcome. We value all user contributions, no matter how minor they are. If we are slow to review, either the pull request needs some benchmarking, tinkering, convincing, etc. or more likely the reviewers are simply busy. In either case, we ask for your understanding during the review process. --> #### PR checklist <!-- Please go through the checklist below. Please feel free to remove points if they are not applicable. --> ##### For all contributions - [ ] I've added myself to the [list of contributors](https://github.com/sktime/sktime/blob/main/CONTRIBUTORS.md) with any new badges I've earned :-) How to: add yourself to the [all-contributors file](https://github.com/sktime/sktime/blob/main/.all-contributorsrc) in the `sktime` root directory (not the `CONTRIBUTORS.md`). Common badges: `code` - fixing a bug, or adding code logic. `doc` - writing or improving documentation or docstrings. `bug` - reporting or diagnosing a bug (get this plus `code` if you also fixed the bug in the PR).`maintenance` - CI, test framework, release. See here for [full badge reference](https://allcontributors.org/docs/en/emoji-key) - [ ] Optionally, I've added myself and possibly others to the [CODEOWNERS](https://github.com/sktime/sktime/blob/main/CODEOWNERS) file - do this if you want to become the owner or maintainer of an estimator you added. See here for further details on the [algorithm maintainer role](https://www.sktime.net/en/latest/get_involved/governance.html#algorithm-maintainers). - [x] The PR title starts with either [ENH], [MNT], [DOC], or [BUG]. [BUG] - bugfix, [MNT] - CI, test framework, [ENH] - adding or improving code, [DOC] - writing or improving documentation or docstrings. ##### For new estimators - [x] I've added the estimator to the API reference - in `docs/source/api_reference/taskname.rst`, follow the pattern. - [x] I've added one or more illustrative usage examples to the docstring, in a pydocstyle compliant `Examples` section. - [x] If the estimator relies on a soft dependency, I've set the `python_dependencies` tag and ensured dependency isolation, see the [estimator dependencies guide](https://www.sktime.net/en/latest/developer_guide/dependencies.html#adding-a-soft-dependency). <!-- Thanks for contributing! -->
sktime#6038) #### Reference Issues/PRs Part of sktime#3351. See also sktime#3365 #### What does this implement/fix? Explain your changes. Migrated CNTC, InceptionTime, and MACNN regressors to sktime from sktime-dl
@fkiraly could you assign this issue to me as I am going to work on it to ensure that all estimators are migrated |
Is your feature request related to a problem? Please describe.
Related to the task of migrating estimators from sktime-dl to sktime. The main goal of doing so is to have all networks/models in a single package, following similar structure and quality.
Steps for Migration
Choose one of the Classifiers or Regressors from the list below, and create an issue for it.
There are 2 key components to a DL Estimator, the Network (creates the actual code of neural network) and the Estimator (Is a layer of abstraction to make the user's life easier).
Lets say I want to create CNNClassifier (steps are similar for any other DL Regressor or Classifier, whereever there are differences, I will mention them)
cnn.py
exists insktime/sktime/networks
. If it does, that means that the network has already been made, and you can skip to the step to create the classifier.cnn.py
insktime/sktime/networks
. This file will contain ourCNNNetwork
. You will have to migrate the network from sktime-dl to sktime. To do so, you will need to go tosktime-dl/networks
and look for_cnn.py
sktime-dl
tosktime
. An example of this migration is that the original code of_cnn.py
from sktime dl has been migrated ascnn.py
in sktime. The code, for most part, remains the same, with different documentation at most. There are a few cases where the migration is not as straightforward, and it would be described in the end.check_estimator
tests as mentioned in the developer guide. Once all the tests pass, you have successfully migrated your Network.cnn.py
atsktime/sktime/classification/deep_learning/
. You will be migrating theCNNClassifier
here. You will have to migrate the network fromsktime-dl
tosktime
. To do so, go tosktime-dl/classification
and look for_cnn.py
.sktime-dl
tosktime
. An example of this migration is that the original code of_cnn.py
fromsktime-dl
has been migrated ascnn.py
insktime
. The code and core logic, for most part, remains the same. There are minor differences in_fit
method you will need to take care of. For other cases where the migration is not as straightforward are described in the end.Special Cases
Adding Soft Dependancy
keras_self_attention
), you have to add them as a soft dependancypyproject.toml
, and check in theall_extras
list whether the dependency you want to add (lets saykeras_self_attention
), already exists within it or not. IF it does, it means that it already is a soft dependancy, and you can skip the steps required to add it as a soft dep.all_extras
list ofpyproject.toml
._check_soft_dependencies
. An example for how it is done is there inCNTCNetwork
DL Classifiers:
InceptionTime
classifier and example (fromsktime-dl
) #3003ResNetClassifier
fromsktime-dl
tosktime
#3881DL Regressors:
ResNetRegressor
fromsktime-dl
#4638The text was updated successfully, but these errors were encountered: