Conversation
| print("Starting training...") | ||
|
|
||
| if self.use_pretrained: | ||
| if self.use_pretrained and \ |
There was a problem hiding this comment.
This if statement can be simplified to:
if self.use_pretrained and (self.use_pretrained_net == use_pretrained_net)
There was a problem hiding this comment.
Also, what does this actually mean? This pull request doesn't change behavior of the embedding matrix right? So why is it required that self.use_pretrained_net == use_pretrained_net ? And why are there two separate variables for use_pretrained_net?
There was a problem hiding this comment.
We only want to initialise the embedding matrix once but we run train twice therefore we want to run it only if both self.use_pretrained_net and use_pretrained_net is true i.e. we want to pre-train on subreddits and this call to train will pre-train or if both of them are false i.e. we don't want to pre-train and this call don't do pre-train.
That's just bad naming, the class variable is whether the model should use pre-training on the network and the parameter is whether this call to the method should pre-train the network
| validation_data: 'validation_data_top_n_single.csv' | ||
| training_data: 'training_data_top_n_single.csv' | ||
| testing_data: 'testing_data_top_n_single.csv' | ||
| pre_train_data: 'pre-train_data_top_n.csv' |
| builder = ModelBuilder(config_file, sess) | ||
| builder.add_input_layer() | ||
|
|
||
| # Add a number of hidden layers |
There was a problem hiding this comment.
Why is this done in main.py? Shouldn't it be done in the build function of the model builder?
There was a problem hiding this comment.
This is more modular, if someone want to set up the network in another way than how it is done in the build method then there is no way of building the model
| .add_precision_operations() | ||
|
|
||
| network_model = builder.build() | ||
| if config_file[USE_PRETRAINED_NET]: |
There was a problem hiding this comment.
Can't this be moved to the model itself? To the beginning of the train function maybe? Just tryting to keep the main.py file as clean as possible 😄
| self._session.run(self.embedding_init, | ||
| feed_dict={self.embedding_placeholder: | ||
| self.data.embedding_matrix}) | ||
| self.train_writer = \ |
There was a problem hiding this comment.
Why are the writers moved to the model builder?
There was a problem hiding this comment.
Perhaps not necessary but otherwise the writer initiated twice, once for each time we train.
|
|
||
| return self | ||
|
|
||
| def add_secondary_output(self): |
There was a problem hiding this comment.
This is very similar to the other output function. Is it possible to combine them and maybe take the labels as an input or something?
There was a problem hiding this comment.
Might be, one problem is that we don't want to overwrite the training op for either could maybe be dealt with using a boolean
| TRAINING = "training_data" | ||
| VALIDATION = "validation_data" | ||
|
|
||
| PRE_TRAINING = "pre_training_data" |
| self.constant_prediction_limit = config[CONSTANT_PREDICTION_LIMIT] | ||
| self.use_concat_input = config[USE_CONCAT_INPUT] | ||
| self.use_pretrained_net = config[USE_PRETRAINED_NET] | ||
| self.subreddit_count = 0 |
|
Also, I just realised that the concatenated subreddit input is still used when pre-training on subreddits. Maybe this should not be concatenated when doing that? Seems a bit biased. At the same time... The dimensions of the weights depend the size of the input. |
|
…hlights/model into feature/pre-train-subreddit
|
Feedback from meeting has been implemented and tested. |
2 participants
This enables to pre-train the network on different output labels for the same input data. Specifically to use subreddits as lables for pre-training.
In main.py the a new layer is added after adding secondary output layer, this is so that the network can generelise from the learnt prameters and when tested showed better preformance. Where in the network the secondary output should be placed should be treated as a hyperparameter.