autonomio / talos

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

9787ff150e185ff5a51a25c69978d2c50621ce79 breaks permutation_filter #257

Closed JohanMollevik closed 5 years ago

JohanMollevik commented 5 years ago

Commit https://github.com/autonomio/talos/commit/9787ff150e185ff5a51a25c69978d2c50621ce79 introduces a bug in permutation filtering which causes the code to not terminate in a timely fassion.

The root cause is that lines 23-29 in ParamGrid should be indented another step to be inside the while loop.

mikkokotila commented 5 years ago

Sorry I don't understand what you refer to. There is no while loop in in ParamGrid and also the lines 23-29 seem to be the to the level of indentation that is possible.

JohanMollevik commented 5 years ago

Sorry, I was looking at diffs and did not see that you moved the code to a new file

correct file is talos/parameters/permutation_filter.py

mikkokotila commented 5 years ago

as of yesterday it's moved to talos/reducers/permutation_filter.py

So it should be like this:

    while len(self.param_grid) < final_grid_size and final_expanded_grid_size < virtual_grid_size:
        final_expanded_grid_size *= 2

        if final_expanded_grid_size > virtual_grid_size:
            final_expanded_grid_size = virtual_grid_size

        self.param_grid = self._create_param_grid(ls,
                                                  final_expanded_grid_size,
                                                  virtual_grid_size)
JohanMollevik commented 5 years ago

No, add two more lines to the loop

mikkokotila commented 5 years ago
    while len(self.param_grid) < final_grid_size and final_expanded_grid_size < virtual_grid_size:
        final_expanded_grid_size *= 2

        if final_expanded_grid_size > virtual_grid_size:
            final_expanded_grid_size = virtual_grid_size

        self.param_grid = self._create_param_grid(ls,
                                                  final_expanded_grid_size,
                                                  virtual_grid_size)

        grid_indices = list(filter(fn, range(len(self.param_grid))))
        self.param_grid = self.param_grid[grid_indices]
mikkokotila commented 5 years ago

Ok I understand, the line numbers had changed as in the current daily-dev the code have been pepified.

JohanMollevik commented 5 years ago

Yes, that looks correct

mikkokotila commented 5 years ago

Great. Closing here.