CodeReclaimers / neat-python

Python implementation of the NEAT neuroevolution algorithm
BSD 3-Clause "New" or "Revised" License
1.41k stars 490 forks source link

Bug in neat.attributes causing _config_items to be a class attribute instead of an instant attribute #188

Open aDecoy opened 4 years ago

aDecoy commented 4 years ago

Hi, the StringAttributes instances have their "options=" parameters overwritten because all StringAttribute objects uses the same memmory location for options as if it was a static variable. This seem to be because it is a class variable, and therefore each instance dont get their own one. (Solution-suggestion at the end) I think this is only a problem when the parameter is a data structure that gets edited, since reasigning the value will make Python turn it into an instance variable.

Place of bug impact: neat.genes line 82 and 83, there is :

                    StringAttribute('activation', options='sigmoid'),
                    StringAttribute('aggregation', options='sum')]

Here the last "options=( a,b,..., sum)" will be used for the one above it also.

Place of bug cause: neat.attributes inside class BaseAttribute line 14 and 15:

    for n, default in default_dict.items():
        self._config_items[n] = [self._config_items[n][0], default]

because self._config_items are not rewrtitten, but only have the n element edited, it remains a class attribute and will be the same for all class instances of StringAttribute. (I am not sure if this problem will exist if only one parameter in options).

Solution: Reasign the memmory spot for the instance, this seem to magicly make _config_items into a instace variable that are unique for that intance and are not shared with other instances/objects of the same class. This can be done with copy.deepcopy(x) My fix was to put in right above line14:

    if hasattr(self,'_config_items') and default_dict:
        # Turn a class attribute into a instance attribute so that it is not shared between all instances of the same class
        # This only exist to turn it from a class attribute to an instance attribute. (class attribute behaves similar to static variables)
        temp_config_items = deepcopy(self._config_items)
        self._config_items = temp_config_items

This seems to solve the problem, but is the options= parameter really necesarry? Should this not be decided by the config file? A better solution could be to just remove it and force users to spesify it in a config file instead of having it hidden away in the code.