Added Model for Time Series Classification

[codeboy5]

I have added the code for a basic RNN Model for the task of time series classification.

> data, blocks = load(datarecipes()["ecg5000"]);
> task = TSClassificationSingle(blocks, data);
> model = FastAI.taskmodel(task);
> traindl, validdl = taskdataloaders(data, task, 32);
> callbacks = [ToGPU(), Metrics(accuracy)];
> learner = Learner(model, tasklossfn(task); data=(traindl, validdl), optimizer=ADAM(), callbacks = callbacks);
> fitonecycle!(learner, 10, 0.033)

As I discussed with @darsnack, the idea is to add an encoding to do the reshaping to (features, batch, time) instead of doing it inside on the RNN Model. Working on that right now.

Have remove the type from StackedLSTM as it was redundant.

[codeboy5]

I have also added a multivariate classification dataset and verified the training loop is working fine for it.

[ToucheSir]

Couple of drive-by comments on the WIP here

[codeboy5]

Hey guys, @ToucheSir @darsnack @lorenzoh. I have updated the notebook. This PR essentially contains code to load regression datasets and the notebook for the classification task.
I will start adding the Inception Time Model in the next PR as we discussed during our weekly call.

[ToucheSir]

Looks good to go modulo a couple small things!

[ToucheSir]

Should be using Flux.outputsize (I think we talked about this in a meeting). Changes required should be minimal :)

[codeboy5]

I tried doing that, but seems to give an error. Will try to debug it later.
https://pastebin.com/mB7aYvdD

[ToucheSir]

I see, this is FluxML/Flux.jl#1755 (comment). Can you do the following then?

• Use zeros or ones instead of rand.
• Pass a batch size of 1 instead of 32 and seq length of 1 instead of inblock.obslength. This will save some time and you don't need it because you only care about the first output dimension.
• reset! the backbone after you calculate output.

[codeboy5]

Yes, sounds great.

[ToucheSir]

Suggested change

Zygote = "e88e6eb3-aa80-5325-afca-941959d7151f"

I don't see Zygote used anywhere, so one less dependency doesn't hurt.

[codeboy5]

I tried doing that, but it throws an error

ERROR: LoadError: ArgumentError: Package FastTimeSeries does not have Zygote in its dependencies:`

[ToucheSir]

This really could use a full stack trace, because AFAICT you're not using Zygote anywhere? Did you ] resolve after removing it from the project?

[codeboy5]

seems to work now, might have missed the resolve. thanks :)

[lorenzoh]

Great progress in my absence and cool to see the training working!

Are the outstanding comments being addressed in #256 or could this be merged individually beforehand?

[codeboy5]

Great progress in my absence and cool to see the training working!

Are the outstanding comments being addressed in #256 or could this be merged individually beforehand?

I am resolving the outstanding comments in #256 , some of them have been resolved and a couple are left, which I will resolve by end of day. This can be merged then I believe.

[codeboy5]

I think this PR is complete ?
Comments have been resolved in the next PR, let me know if something is missing.

[ToucheSir]

I wasn't aware #256 was also working on the StackedRNN model. It would be preferable to merge those changes here so that PR stays more focused on the InceptionTime. If not, we can just close this one and focus on that one (I'm interpreting your comment to mean that the changes in #256 completely subsume this PR).

[codeboy5]

I wasn't aware #256 was also working on the StackedRNN model. It would be preferable to merge those changes here so that PR stays more focused on the InceptionTime. If not, we can just close this one and focus on that one (I'm interpreting your comment to mean that the changes in #256 completely subsume this PR).

PR #256 does contain the code changes in this PR, since I created that branch from this branch. Other than that, just some renaming that's done there, etc.
If we merge this, would those commits not be committed in that PR (since those commits are already committed to the master branch ) ?
Specifically Commits from added RNN Model to updated notebook.
If yes, we can merge this and then merge that one. Otherwise we can close this and merge that one.

[ToucheSir]

You'd likely have to rebase #256 after merging this PR if you've made changes to the same functions/types in that one. If that doesn't sound like a problem, we can merge this first.

[codeboy5]

You'd likely have to rebase #256 after merging this PR if you've made changes to the same functions/types in that one. If that doesn't sound like a problem, we can merge this first.

Wouldn't be an issue. 👍🏻

[ToucheSir]

Merging as noted to follow up in #256.