bmoore20 / habs

Detect Harmful Algal Blooms (HABs) in images of the Finger Lakes.
0 stars 0 forks source link

Feature/configure abstract #39

Closed bmoore20 closed 3 years ago

bmoore20 commented 3 years ago

Hey Ty! Can you please take a look at these changes when you get the chance and let me know what you think? Thanks! @AlephNotation

This PR includes two main changes:

  1. Configuration of the hyperparameters (#37)
  2. Abstraction of the training and testing loops (#38, #17 )

I combined them into one PR because their changes are dependent on each other.

For the configuration of the hyperparameters, I created a new file under the utils folder called selectors. For the hyperparameters that were objects (model, criterion, and optimizer) I needed a way to initiate the specific type of objects that the user passed in before I gave them to the train method call. Pulling this functionality out into a separate file was the cleanest option. All of the selector calls are called in main in train.py. Currently, each selector only has one option, so it seems like a little bit of an overkill. But I am anticipating that once we start experimenting with the model, we will want to add more options to these selectors. With this design in will be easy to add in new types of models, optimizers, and criterions.

For the abstraction of the training and test loops, I created a new file under the utils folder called training_helper. This file consists of two methods, training_laps and evaluate. training_laps consists of the functionality that trains the dataset, while evaluate pulls out the code that handles the evaluation of the trained model. Both are called in train in train.py. By pulling out the training and testing loops, train.py now has two main jobs: 1) load in the data 2) call the train and test functions. Overall, train.py is now a lot easier to read and follow.

bmoore20 commented 3 years ago

Thanks for the great feedback @AlephNotation !

Also - thanks for the reminder about using Black. I just set it up with the repo using pre-commits so it should be all set now! I submitted and merged the PR (#41) for it so it's in main already.

bmoore20 commented 3 years ago

@AlephNotation I am going to merge the code in this PR because we went through all of the revised changes that I made during our call yesterday and you said everything looked good. The commits I made today were all little changes that you suggested yesterday. A list of these changes are:

Time to FINALLY run the program!! Thanks for your help!