Backwards compatibility for Missing Training Args

[cjmcgill]

Fix for #148. This PR adds a function that fills in the default values for training args when those training args are missing in the checkpoint file. This bridges a backwards compatibility issue introduced at #137.

[lgtm-com]

This pull request introduces 2 alerts when merging d874a0f into c60973b - view on LGTM.com

new alerts:

• 2 for Unused local variable

[cjmcgill]

Have tested this with multiple molecules and with molecule features.

[hesther]

I think this is great (especially to have all the checks in a separate functions, since this also cleans up large portions in make_predictions.py and molecule_fingerprints.py. Only concern: Do the default values of newly introduced arguments correspond to the previous behavior? For example, in the recent atom and bond_feature PR, we introduced scaling for atom and bond features, which defaults to True. However, the old behavior was not to scale, and there was no argument for scaling, so the default of the new argument would not correspond to the previous behavior. I am not sure whether we should worry about this, since this feature was only up for a few months without scaling, but wanted to mention it anyways. I am not sure whether there are more cases like this, too.

[cjmcgill]

@hesther I think this is an important concern and it's one that I was trying to think about as well. It's one of the reasons I was looking so close at continuity yesterday.

My only problem was that I couldn't find an example where the default wasn't the old behavior. I was using only fairly basic arguments in the search so it makes sense that I overlooked the atom features scaling.

My solution to this would be (now and in the future) to make a hard override list in the function to use instead of defaults if the default behavior is not the previous behavior. This should be a rare occurrence. I'll go and add the feature scaling argument to such a list in the function. If we can identify any others like this, we should add them as well.

[cjmcgill]

The atom and bond features scaling (original args and intermediate value args) are included now in the override list.

It occurs to me that necessary intermediate value args may not be caught in the current structure, if the argument is not set during init or process_args. This will be a gap in the backwards compatibility (error causing, not wrong value) if things depend on intermediate argument values. But I think the override list is the right solution to this as well.

[cjmcgill]

@mliu49 this fix should be ready to merge

[mliu49]

I added a bunch of minor comments and suggestions. I think the code consolidation is great!

[cjmcgill]

@mliu49 thanks again for all the good notes! They should all be implemented now.