Skip to content

[ENH] migrate MCDCNN classifier, regressor, network from sktime-dl #4637

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 11 commits into from
Jul 14, 2023
Merged

[ENH] migrate MCDCNN classifier, regressor, network from sktime-dl #4637

merged 11 commits into from
Jul 14, 2023

Conversation

achieveordie
Copy link
Collaborator

Reference Issues/PRs

Fixes #3295
Part of #3351

What does this implement/fix? Explain your changes.

Add MCDCNNClassifier, MCDCNNRegressor and MCDCNNNetwork

Does your contribution introduce a new dependency? If yes, which one?

None

What should a reviewer concentrate their feedback on?

Did you add any tests for the change?

Any other comments?

PR checklist

For all contributions
  • I've added myself to the list of contributors with any new badges I've earned :-)
    How to: add yourself to the all-contributors file 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
  • Optionally, I've added myself and possibly others to the 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.
  • 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
  • I've added the estimator to the API reference - in docs/source/api_reference/taskname.rst, follow the pattern.
  • I've added one or more illustrative usage examples to the docstring, in a pydocstyle compliant Examples section.
  • If the estimator relies on a soft dependency, I've set the python_dependencies tag and ensured
    dependency isolation, see the estimator dependencies guide.

@achieveordie achieveordie requested a review from fkiraly as a code owner May 23, 2023 06:50
@achieveordie
Copy link
Collaborator Author

@fkiraly could you please review this too?

fkiraly
fkiraly reviewed Jun 8, 2023
View reviewed changes
fkiraly
fkiraly reviewed Jun 8, 2023
View reviewed changes
fkiraly
fkiraly previously approved these changes Jun 8, 2023
View reviewed changes
Copy link
Collaborator

@fkiraly fkiraly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks - left some non-blocking comments, but would appreciate a quick look.

@achieveordie
Copy link
Collaborator Author

Currently, Ubuntu-3.11 is failing due to:

FAILED sktime/forecasting/base/tests/test_fh.py::test_auto_arima - ValueError: Could not successfully fit a viable ARIMA model to input data.
See http://alkaline-ml.com/pmdarima/no-successful-model.html for more information on why this can happen.

Is this a known issue? @yarnabrina @fkiraly

@yarnabrina
Copy link
Member

yarnabrina commented Jun 10, 2023 •
edited
Loading

Not known to me, and cannot reproduce locally on main (on commit 9544d32).

Command used:

pytest -k "test_auto_arima" sktime/forecasting/base/tests/test_fh.py

Versions:

>>> from sktime import show_versions; show_versions()

System:
    python: 3.11.3 (main, Apr 19 2023, 23:54:32) [GCC 11.2.0]
executable: /path/to/venv/bin/python
   machine: Linux-5.15.90.1-microsoft-standard-WSL2-x86_64-with-glibc2.35

Python dependencies:
          pip: 22.3.1
       sktime: 0.19.1
      sklearn: 1.2.2
        numpy: 1.24.3
        scipy: 1.10.1
       pandas: 2.0.2
   matplotlib: None
       joblib: 1.2.0
  statsmodels: 0.14.0
        numba: None
     pmdarima: 2.0.3
      tsfresh: None
   tensorflow: None
tensorflow_probability: None

@achieveordie achieveordie mentioned this pull request Jun 11, 2023
Open
@fkiraly fkiraly added the module:classification classification module: time series classification label Jul 13, 2023
fkiraly
fkiraly previously approved these changes Jul 13, 2023
View reviewed changes
Copy link
Collaborator

@fkiraly fkiraly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, we should be able to merge.

@fkiraly fkiraly changed the title [ENH] Add MCDCNN Estimator [ENH] migrate MCDCNN classifier, regressor, network from sktime-dl Jul 14, 2023
@fkiraly fkiraly added the implementing algorithms Implementing algorithms, estimators, objects native to sktime label Jul 14, 2023
@fkiraly fkiraly merged commit 0c8b8a1 into sktime:main Jul 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fkiraly fkiraly fkiraly left review comments

@benHeid benHeid Awaiting requested review from benHeid benHeid is a code owner

@yarnabrina yarnabrina Awaiting requested review from yarnabrina yarnabrina is a code owner

Assignees
No one assigned
Labels
implementing algorithms Implementing algorithms, estimators, objects native to sktime module:classification classification module: time series classification
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[ENH] Adding MCDNN DL model for Classification
3 participants
@achieveordie @yarnabrina @fkiraly