autonomio / talos

Hyperparameter Experiments with TensorFlow and Keras
https://autonom.io
MIT License
1.62k stars 268 forks source link

Best model is not saved properly #17

Closed matthewcarbone closed 6 years ago

matthewcarbone commented 6 years ago

This is referenced in e6ecae7a74d9912bc66a14607e9e87b9a072c2ab but seems to not have been resolved. I also cannot find the commit where Scan.py was changed to remove the additions from that commit. Figured it made sense to open an issue about it.

Current implementation runs successfully:

h = ta.Scan(np.concatenate((x_train, x_dev)),
            np.concatenate((y_train, y_dev)),
            params=p,
            dataset_name='xray',
            experiment_no='1',
            model=general_model,
            grid_downsample=0.0000025,  # lots of parameters
            val_split=0.2,
            save_best_model=True)

# output
# 20 scans will take roughly 160 seconds
# Scan Finished!

But when attempting to load

from keras.models import load_model
mod = load_model('xray_1.h5')

raises

/usr/local/lib/python3.6/dist-packages/keras/models.py in load_model(filepath, custom_objects, compile)
    266         model_config = f.attrs.get('model_config')
    267         if model_config is None:
--> 268             raise ValueError('No model found in config file.')
    269         model_config = json.loads(model_config.decode('utf-8'))
    270         model = model_from_config(model_config, custom_objects=custom_objects)

ValueError: No model found in config file.

Has this been resolved already and I'm just missing something?

mikkokotila commented 6 years ago

@x94carbone I worked this out but found that there was quite a bit of discrepancy in the way various sources said was the right way to handle it, and did not have time to test validity of the implementation I did properly so I left it out for now. There is also the case of how to really handle this beyond just the code implementation.

For example you might end up with 10-20 configurations with close to identical result. So it seems there is a need rather to add another layer of automation, where a configurable number of best results is picked, and then run again in a meaningful process that picks the best one. This leads to the question of "best" in terms of a well generalizing model vs. test performance.

The way I found the process tends to play out is something like...start with something pretty broad, and end up with some ideas on how to improve. Then narrow the boundaries, and end up with some ideas on how to improve even more. Maybe repeat this a few times, depending on the prediction task, and end up with a bunch of well performing configurations. So this will be another useful layer of process automation but involves more open questions than the one mentioned above. So I'm a little inclined to skip the "legacy" approach of picking the best model all together at least for now and go straight to figuring out an approach that appreciates the problem more.

In which case the parameter save_best_model is redundant for now.

matthewcarbone commented 6 years ago

@mikkokotila Thanks for the response! I have some ways in mind that could be used address these concerns as well. They pertain closely to my current research. What it appears you're talking about is a more insightful way of picking the best hyperparameters (HP). I have two ideas about this depending on how far you're willing to take this.

First, the first step could be exactly what Talos already does: screen different HP configurations, but then pick the top N of them. Second, of those top N models, randomly permute the test/train/dev sets and retrain/eval for each permutation. Implement some measure of model robustness such that only models that are robust to the random permutations of the data are kept and return the best overall. This also might involve a more thoughtful split of the data initially so as to avoid bias.

Second, you (we?) should consider evolutionary algorithms. This will greatly improve performance overall and could be built on top of the first step.

But anyway regarding the actual issue here I understand what you're saying. Is there any way to completely remove the option and prior package (Hyperio) to avoid confusion?

mikkokotila commented 6 years ago

@x94carbone 👍

Regarding your first idea, I agree. Maybe there is an option to pick top N configurations / all configurations that meet a set metric threshold. It will probably be a good idea to always throw in some randomly or otherwise picked configurations for control purpose.

Regarding the split of the data, I think the starting point would be that a "large as possible" portion of the original dataset is reserved for the Val() stage and it never touch the Scan() stage. Then split further in some meaningful way when it gets to Val(). It seems that the splitting part will be trivial and not affecting the mainline of Val() so might be better to handle it separately later. The other thing is to have all configurations in Val() run n times and measure std, mean and max as evaluation metric as opposed to just the objective metric (e.g. f1 score).

Regarding evolutionary algorithms. I think that is a great idea. After having lightly reviewed the many approaches used for this problem, I thought the evolutionary algorithm approach was very interesting. Are you familiar with the python libraries for that purpose? Do you have suggestions for how to best approach adding it. I think it would be a good idea to prioritize this as it makes the current iterative approach redundant and allows shifting focus to the next layer of process automation. I'll create a separate issue on that with some related questions.

[EDIT] There is now #18 for the evolutionary algorithm topic.

mikkokotila commented 6 years ago

For the actual issue, I will remove all the redundant codes.

matthewcarbone commented 6 years ago

@mikkokotila Sounds good. I absolutely agree on all points. I'll comment in #18 about evolutionary algorithms and elsewhere where necessary.

mikkokotila commented 6 years ago

The artifacts were cleaned from the current version.

The wider discussion continues in #40 so closing here.