DeepTrackAI / deeplay

Other
5 stars 6 forks source link

add recurrent neural network #38

Closed JesusPinedaC closed 9 months ago

JesusPinedaC commented 11 months ago

This pull request introduces recurrent neural networks based on @Henrik-KM 's implementation in https://github.com/DeepTrackAI/deeplay/pull/23

Example:

rnn = dl.RecurrentNeuralNetwork(
    in_features=2,
    hidden_features=[32, 32],
    out_features=10
)

Every component of the network is fully flexible. For instance:

rnn.blocks.layer.configure(nn.LSTM)

Or, in the case of bidirectional RNNs:

rnn.blocks.layer.configure(bidirectional=True)

# The forward and backward directions are concatenated by the RNN layer, so the input_size of the hidden layers needs to be updated
rnn.blocks[1:].layer.configure(input_size=64)

@Henrik-KM, the idea is updating this implementation based on your inputs!

giovannivolpe commented 11 months ago

@BenjaminMidtvedt @Henrik-KM there are some conflicts that need to be resolved before merging this. Can you check and confirm we can proceed to merge?

cmanzo commented 10 months ago

@JesusPinedaC @Henrik-KM I'm having some trouble with the following code:

import deeplay as dl
from torch.nn import CrossEntropyLoss

rnn = dl.RecurrentNeuralNetwork(
    in_features=n_features,
    hidden_features=[100],
    out_features=model_number,
)
rnn.blocks.dropout.configure(p=0.2)

model_rnn = dl.Classifier(
    model=rnn,
    loss=CrossEntropyLoss(),
    optimizer=dl.Adam(lr=0.001),
)
model_rnn.create()

trainer_rnn = dl.Trainer(max_epochs=30, accelerator="auto")
trainer_rnn.fit(model_rnn, train_loader)

I get MisconfigurationException: Unknown configuration for model optimizers. Am I doing anything wrong?

JesusPinedaC commented 10 months ago

@JesusPinedaC @Henrik-KM I'm having some trouble with the following code:

import deeplay as dl
from torch.nn import CrossEntropyLoss

rnn = dl.RecurrentNeuralNetwork(
    in_features=n_features,
    hidden_features=[100],
    out_features=model_number,
)
rnn.blocks.dropout.configure(p=0.2)

model_rnn = dl.Classifier(
    model=rnn,
    loss=CrossEntropyLoss(),
    optimizer=dl.Adam(lr=0.001),
)
model_rnn.create()

trainer_rnn = dl.Trainer(max_epochs=30, accelerator="auto")
trainer_rnn.fit(model_rnn, train_loader)

I get MisconfigurationException: Unknown configuration for model optimizers. Am I doing anything wrong?

@cmanzo Doing

model_rnn = dl.Classifier(
    model=rnn,
    loss=CrossEntropyLoss(),
    optimizer=dl.Adam(lr=0.001),
).create()

instead, solves the issue!

cmanzo commented 10 months ago

It seems that:

rnn = dl.RNN(
    in_features=4,
    hidden_features=[100],
    out_features=5,
    rnn_type="RNN",
    dropout=0.2,
)

doesn't set the dropout correctly.

However, rnn.blocks.dropout.configure(p=0.2) does work.

There is also some inconsistency when printing the models. For example print(rnn) displays:

RNN(
  (blocks): LayerList(
    (0): RecurrentNeuralNetwork(
  ...
          (dropout): Layer[RecurrentDropout](p=0.2)
        )
...
    (1): MultiLayerPerceptron(
...
          (dropout): Layer[Dropout](p=0.2)
        )

but

model_rnn = dl.Classifier(
    model=rnn,
    loss=torch.nn.CrossEntropyLoss(),
    optimizer=dl.Adam(lr=0.001),
).create()
print(model_rnn.model)

produces:

RNN(
  (blocks): LayerList(
    (0): RecurrentNeuralNetwork(
      (blocks): LayerList(
        ...
          (dropout): RecurrentDropout()
        )
 ...
      (blocks): LayerList(
        (0): LayerActivationNormalizationDropout(
 ...
          (dropout): Dropout(p=0.2, inplace=False)
        )
      )

However, the value is set correctly as it can be verified by:

model_rnn.model.blocks[0].blocks[0].dropout.p

BenjaminMidtvedt commented 9 months ago

@JesusPinedaC @Henrik-KM can you check the merge conflicts?

Henrik-KM commented 9 months ago

@JesusPinedaC @Henrik-KM can you check the merge conflicts?

I think this PR can be closed, right @JesusPinedaC? All the changes should have been implemented in the separate RNN PR that was merged a few days ago.

JesusPinedaC commented 9 months ago

@JesusPinedaC @Henrik-KM can you check the merge conflicts?

I think this PR can be closed, right @JesusPinedaC? All the changes should have been implemented in the separate RNN PR that was merged a few days ago.

I think it was not merged into develop @Henrik-KM

JesusPinedaC commented 9 months ago

@Henrik-KM @BenjaminMidtvedt Should we add unittests to test the new model?

BenjaminMidtvedt commented 9 months ago

@JesusPinedaC yes. Should formalize exactly the things to test but check the tests by carlo as a reference