p-christ / nn_builder

Build neural networks with less boilerplate code
MIT License
162 stars 22 forks source link

Fix GPU compat, Question on NN __init__ #6

Closed josiahls closed 5 years ago

josiahls commented 5 years ago

Section 1 of 2

When GPU is on, assert isinstance(x, torch.FloatTensor) will fail since GPU tensors are not torch.FloatTensor. This is fixed by adding: self.check_input_data_into_forward_once(x.cpu()) However, perhaps it would be better to try: assert isinstance(x.cpu(), torch.FloatTensor) ?

Section 2 of 2

There are parameter conflicts during initialization. Note in https://github.com/p-christ/Deep-Reinforcement-Learning-Algorithms-with-PyTorch/blob/80c09bdac501af3bc47d74901ceadd2f778cf3cb/agents/Base_Agent.py#L310 the initialization is:

return NN(input_dim=input_dim, hidden_layers_info=hyperparameters["linear_hidden_units"],
                  output_dim=output_dim, output_activation=hyperparameters["final_layer_activation"],
                  batch_norm=hyperparameters["batch_norm"], dropout=hyperparameters["dropout"],
                  hidden_activations=hyperparameters["hidden_activations"], initialiser=hyperparameters["initialiser"],
                  columns_of_data_to_be_embedded=hyperparameters["columns_of_data_to_be_embedded"],
                  embedding_dimensions=hyperparameters["embedding_dimensions"], y_range=hyperparameters["y_range"],
                  random_seed=seed).to(self.device)

However the current initialization is:

    def __init__(self, input_dim: int, layers_info: list, output_activation=None,
                 hidden_activations="relu", dropout: float =0.0, initialiser: str ="default", batch_norm: bool =False,
                 columns_of_data_to_be_embedded: list =[], embedding_dimensions: list =[], y_range: tuple = (),
                 random_seed=0, print_model_summary: bool =False)

Two issues:

So we can either: a. Change Deep-Reinforcement-Learning-Algorithms-with-PyTorch code to match this b. Change this to match Deep-Reinforcement-Learning-Algorithms-with-PyTorch.

Currently I changed a little bit of both, however I like how Deep-Reinforcement-Learning-Algorithms-with-PyTorch natively inputs layer information. I guess I am currious about the rational behind the differences between the 2. Based on your response, I might change this to better match Deep-Reinforcement-Learning-Algorithms-with-PyTorch's usage of this library. I like having a parameter for input, hidden, and output separate.

One advantage of a single input "layer_info" might be the greater flexibility of different layers. Possibly supporting list, or dictionary inputs in the future.

p-christ commented 5 years ago

Great, thanks a lot! Deep-Reinforcement-Learning-Algorithms-with-PyTorch is just using an out of date version off nn_builder. It should be layers_info in both, i will change it