FluxML / FastAI.jl

Repository of best practices for deep learning in Julia, inspired by fastai
https://fluxml.ai/FastAI.jl
MIT License
585 stars 51 forks source link

Add Time Series Block #239

Closed codeboy5 closed 2 years ago

codeboy5 commented 2 years ago

Added Time Series Container and Block. It is capable of loading all datasets from timeseriesclassification. The .ts files are loaded using Julia translation of this method .

I have also added a basic test case for the recipe. This allows us to do the following

using FastAI

data, blocks = load(datarecipes()["ecg5000"])
nobs(data)
sample = series, class = getobs(data, 10)

Just wanted to get some initial thoughts on the work, there might be more changes as I continue to work on the other parts.

codeboy5 commented 2 years ago

Hey for downloading the datasets, the message as well as the post fetch method needs to be different which I think might require some restructuring, how should I go about that ?

codeboy5 commented 2 years ago

I have implemented the changes we discussed in the last call.

ToucheSir commented 2 years ago

Great. Just to be safe, I think it would be best to download the file on the fly for now.

codeboy5 commented 2 years ago

I have added the basic structure and code. There seems to be a couple of errors, I will solve by tonight. Let me know how does the basic code look.

codeboy5 commented 2 years ago

Hey the encodings are working now.

> using FastAI
> data, blocks = load(datarecipes()["ecg5000"]);
> task = FastAI.TSClassificationSingle(blocks, data);
> input, target = getobs(data, 2)
(Float32[-1.1008778 -3.9968398 โ€ฆ 1.1196209 -1.4362499], "1")
> encodesample(task, Training(), (input, target))
(Float32[-1.1048299 -4.011188 โ€ฆ 1.1236403 -1.4414059], Float32[1.0, 0.0, 0.0, 0.0, 0.0])

I am looking to add some tests as well look to resolve all the comments you guys made before the meeting. Have also started working on creating the demo notebook side-by-side as you guys suggested.

lorenzoh commented 2 years ago

Could you push the notebook here as well? You can just place it in the notebooks folder

lorenzoh commented 2 years ago

Since I merged #240, this shows a lot of merge conflicts, but don't worry, I will do the merge myself manually once this PR is ready ๐Ÿ‘

codeboy5 commented 2 years ago

I've added the decode method and basic visualisation too and updated the notebook accordingly. The formatting for plots Is a little weird on the notebooks, but looks fine on the terminal. Do you guys know how to fix this, should I try adjusting the size of the plot ?

Screenshot 2022-07-06 at 10 07 39 AM
lorenzoh commented 2 years ago

The formatting for plots Is a little weird on the notebooks, but looks fine on the terminal.

Rendering of Unicode plots can be a bit off depending on the notebook environment/viewer. It should end up looking fine in the docs, though.

For regular usage, the Makie backend will probably preferable, but we can get to that later :+1:

lorenzoh commented 2 years ago

Having some trouble to push the subpackage merge to your fork, @codeboy5. Can you check that I have permissions to push to your fork? Or, alternatively just give me committer access to the fork

image

codeboy5 commented 2 years ago

Having some trouble to push the subpackage merge to your fork, @codeboy5. Can you check that I have permissions to push to your fork? Or, alternatively just give me committer access to the fork

image

If checked, users with write access to FluxML/FastAI.jl can add new commits to your timeseries-blocks branch. You can always change this setting later. This option is checked out for me. Should I add you as a collaborator on my forked repo ?

lorenzoh commented 2 years ago

Having some trouble to push the subpackage merge to your fork, @codeboy5. Can you check that I have permissions to push to your fork? Or, alternatively just give me committer access to the fork

image

If checked, users with write access to FluxML/FastAI.jl can add new commits to your timeseries-blocks branch. You can always change this setting later. This option is checked out for me. Should I add you as a collaborator on my forked repo ?

Yeah, please try that

codeboy5 commented 2 years ago

Having some trouble to push the subpackage merge to your fork, @codeboy5. Can you check that I have permissions to push to your fork? Or, alternatively just give me committer access to the fork image

If checked, users with write access to FluxML/FastAI.jl can add new commits to your timeseries-blocks branch. You can always change this setting later. This option is checked out for me. Should I add you as a collaborator on my forked repo ?

Yeah, please try that

Yeah, just sent you an invite for the same. Hopefully it should work now.

lorenzoh commented 2 years ago

I merged this PR into master, so it now lives in the subpackage FastTimeSeries. The CI is failing right now for unrelated reasons, I'll rerun it when those are fixed (by #247)

codeboy5 commented 2 years ago

With this commit, we are able to the following now

> using FastAI, FastTimeSeries
> data, blocks = load(datarecipes()["ecg5000"]);
> task = FastTimeSeries.TSClassificationSingle(blocks, data);
> backbone = FastTimeSeries.Models.StackedLSTM(1, 16, 10, 2);
> model = FastAI.taskmodel(task, backbone)
Chain(
  StackedLSTMCell(
    Recur(
      LSTMCell(1 => 10),                # 500 parameters
    ),
    Recur(
      LSTMCell(10 => 16),               # 1_760 parameters
    ),
  ),
  identity,
  Dense(16 => 5),                       # 85 parameters
)         # Total: 12 trainable arrays, 2_345 parameters,
          # plus 4 non-trainable, 13_312 parameters, summarysize 62.145 KiB.
codeboy5 commented 2 years ago
image
lorenzoh commented 2 years ago

Since this PR is getting quite long, it may be best to move the newest changes to a new PR and merge the parts that are ready. We could revert the PR to the point where I did the manual merge, merge this PR with the changes up to that point into master, and then open a new PR based on master with the more recent changes, including models and so on. @codeboy5 let me know if you arenโ€™t sure about how to do this ๐Ÿ™‚

codeboy5 commented 2 years ago

Since this PR is getting quite long, it may be best to move the newest changes to a new PR and merge the parts that are ready. We could revert the PR to the point where I did the manual merge, merge this PR with the changes up to that point into master, and then open a new PR based on master with the more recent changes, including models and so on. @codeboy5 let me know if you arenโ€™t sure about how to do this ๐Ÿ™‚

Yeah sure, just have to revert the code to the pr right ? Will do it.

codeboy5 commented 2 years ago

@lorenzoh Does this look okay ?

On the last meeting, @darsnack suggested some changes to the model code, I will commit them in the next PR.

lorenzoh commented 2 years ago

Yeah this looks good! Just to make sure, @darsnack does it make sense to rebase this on master when merging or should we squash and merge?

darsnack commented 2 years ago

Either is okay. Maybe squash and merge will be less painful?

lorenzoh commented 2 years ago

I've squashed and merged it. @codeboy5 you should be able to open a new PR for the model code based on master now.

codeboy5 commented 2 years ago

I've squashed and merged it. @codeboy5 you should be able to open a new PR for the model code based on master now.

will do thanks . ๐Ÿ‘๐Ÿป