Skip to content
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

Dev #19

Merged
merged 313 commits into from Sep 3, 2020
Merged

Dev #19

merged 313 commits into from Sep 3, 2020

Conversation

james-large
Copy link
Contributor

Long time no PR, getting this into master before working on new things. Proposing that this updates sktime-dl to 0.2.0

See #14 for most of the changes.

Major:

  • added the inception time TSC model (https://github.com/hfawaz/InceptionTime)
  • introduction of meta package on top level, housing generic ensemble and tuning classes and their tests
  • capability to save trained models added (back in, to some extent). Default off
  • readme semi-overhaul (somewhat preempting setup.py changes, see below)

Also included are some updates to the setup.py and travis tests.

@mloning
Copy link
Contributor

mloning commented Jan 27, 2020

Let's wait for the regressor refactoring on #22 before me make a new release!

@Withington
Copy link
Collaborator

The pull request build failure above failed with ValueError: Error when checking target: expected dense_9 to have shape (1,) but got array with shape (2,)
I've seen this a few times on MCDCNN on other branches and with TensorFlow 1. It isn't an issue unique to this Pull Request.

@Withington
Copy link
Collaborator

The dev branch build on my fork passed -
https://travis-ci.com/Withington/sktime-dl/builds/146884445

…base classifier to base/estimators/_classifier.py. Tests pass.
…ase regressor to base/estimators/_regressor.py. Tests pass.
…e (e.g. a network without its final layer) into deeplearning/[architecture name]. Moved base network to base/estimators/_network.py. Tests pass.
…them to the network, classifier, regressor structure already used for CNN and MLP.
…t data moved to adjust_parameters(X) and extra adjustment added to handle the short series that might be used for forecasting. Modified slice_data() to handle y that can be regression values or int class labels and where y can be 1D array (regression) or 2D array (classification). Modified pre_processing() to handle y that can be 1D array or 2D array.
Withington and others added 23 commits August 13, 2020 17:19
* fixed forecasting test for time series regressors

* Update test_regressors.py

Updated test to reflect that the model lists are dicts now

* Removing MCDCNNRegressor_quick from model lists (and so tests) to confirm other tests complete successfully

* MCDCNNRegressor seems to need extra epochs

* including mcdcnn back into the model lists

* Update test_regressors.py

Co-authored-by: James Large <44509982+James-Large@users.noreply.github.com>
…sle for one particular test case just because its not getting enough data
@james-large
Copy link
Contributor Author

My lord, the tests passed. Validation tests have been marked as slow and not to run on travis, as too often there's one or two envs where it hits the 10 min time out while the rest are fine.

After release, there'll be some issues put up and we'll do a roadmap post for 0.3.0 in the future, one of them will be revamping the tests and streamlining them. Part of me dreads merging this and then one of the tests fails due to bad initialisation again on master

Either way, think this is a long over due merge. Good to submit reviews?

@Withington
Copy link
Collaborator

Thanks for persevering @james-large. Good to merge 👍

Copy link
Contributor

@mloning mloning left a comment

Choose a reason for hiding this comment

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

Yes, I'm happy to merge too!

@mloning
Copy link
Contributor

mloning commented Aug 26, 2020

Sorry for the delay!

@james-large james-large merged commit feb42cf into master Sep 3, 2020
@Withington
Copy link
Collaborator

Fantastic to see this merge go through 🥳
What is the next step to get it released on pypi?
@james-large @mloning

@mloning
Copy link
Contributor

mloning commented Sep 8, 2020

@Withington I don't think we've set up the CI so that tags automatically trigger uploads to pypi. Implementing that would be really helpful. The sktime base repo could serve as an example. Otherwise we can do it by downloading the build artefacts and manually uploading them pypi.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants