Out of bound (IndexError) in param_space when using boolean_limit

oubathAI commented 3 years ago

1) Confirm the below

[x] My Python version is 3.5 or higher
[x] I have searched through the issues Issues for a duplicate
[x] I've tested that my Keras model works as a stand-alone

2) Include the output of:

talos.__version__ : 1.0.0

3) Explain clearly what you expect to happen

Execute a Scan using boolean_limit option without crashing

4) Explain what actually happened

When using boolean_limit option, Scan fails because of out of bounds error in param_space when reduction is applied.

IndexError                                Traceback (most recent call last)
<ipython-input-3-64df655e1857> in <module>
     16     clear_session=True,
     17     save_weights=False,
---> 18     boolean_limit=lambda params: params["neurons"] < 500
     19 )

~/anaconda3/lib/python3.7/site-packages/talos/scan/Scan.py in __init__(self, x, y, params, model, experiment_name, x_val, y_val, val_split, random_method, seed, performance_target, fraction_limit, round_limit, time_limit, boolean_limit, reduction_method, reduction_interval, reduction_window, reduction_threshold, reduction_metric, minimize_loss, disable_progress_bar, print_params, clear_session, save_weights)
    194         # start runtime
    195         from .scan_run import scan_run
--> 196         scan_run(self)

~/anaconda3/lib/python3.7/site-packages/talos/scan/scan_run.py in scan_run(self)
     17 
     18         # get the parameters
---> 19         self.round_params = self.param_object.round_parameters()
     20 
     21         # break when there is no more permutations left

~/anaconda3/lib/python3.7/site-packages/talos/parameters/ParamSpace.py in round_parameters(self)
    172 
    173                 # get the values based on the index
--> 174                 values = self.param_space[index]
    175                 round_parameters = self._round_parameters_todict(values)
    176 

IndexError: index 5435 is out of bounds for axis 0 with size 5435

5) Provide a code-complete reference

[x] My bug report includes an input model
[x] My bug report includes a parameter dictionary
[x] My bug report includes a Scan() command
[x] My bug report question includes a link to a sample of the data

NOTE: If the data is sensitive and can't be shared, create dummy data that mimics it.

A self-contained Jupyter Notebook, Google Colab, or similar is highly preferred and will speed up helping you with your issue.

I've simplified the workflow to reproduce the error faster. Please find it on Google Colab: Google Colab: https://colab.research.google.com/drive/1zTJUiSr3OiIfQDFp-R4AaoOxhajkjTX2?usp=sharing I can upload the code here if it's better.

github-actions[bot] commented 3 years ago

Welcome to Talos community! Thanks so much for creating your first issue :)

oubathAI commented 3 years ago

I started to look at the code and for the moment it seems that the error comes from this line : https://github.com/autonomio/talos/blob/dbd27b46a019f6fbfb7b7f08b2eb0de3be0a41ad/talos/parameters/ParamSpace.py#L52

Instead of having self.param_index = list(range(len(self.param_index)))) it seems to me that we should have self.param_index = list(range(len(self.param_space)))))

Can you confirm if I'm right?

mikkokotila commented 3 years ago

It seems that the above message proposes to add a trailing ) but nothing else?

Moreover, self.param_index = list(range(len(self.param_index))) is doing what we want it to do i.e. reset the parameter index. Did you see this happen often, or just once?

oubathAI commented 3 years ago

It seems that the above message proposes to add a trailing ) but nothing else?

No, it's not about trailing ). Actually, I made a mistake when I made a copy/paste of the code, a mistake which added more ) . I was proposing to replace self.param_index = list(range(len(self.param_index))) by self.param_index = list(range(len(self. param_space))) ; i.e I propose to reinitialize param_indexusing new value of param_space , since param_space had been modified when boolean_limit was applied.

Moreover, self.param_index = list(range(len(self.param_index))) is doing what we want it to do i.e. reset the parameter index.

I get it; but since param_space is been modified when applying boolean_limit: https://github.com/autonomio/talos/blob/dbd27b46a019f6fbfb7b7f08b2eb0de3be0a41ad/talos/parameters/ParamSpace.py#L46-L52

It seems that param_index should be reinitialized using the new param space to avoid out of bound error.

Did you see this happen often, or just once?

It happens almost all the time when I execute a scan. Did you have the time to check the Google Colab I shared? The error is quite easily reproducible is less than 2 min.

I've made the change I proposed locally and I can confirm there's no more issue with this modification.

mikkokotila commented 3 years ago

Makes sense. Thanks for pointing it out clearly.

Do you want to open a PR for it?

oubathAI commented 3 years ago

Oh it's just a line to change; no need a PR for this. I see you update talos regularly, so I let you update it with your next commit. Have a nice weekend !

InigoMoreno commented 3 years ago

Was this ever fixed? I just had the same issue and looking at the master branch it seems like the change @oubathAI proposed was never applied.

autonomio / talos