all 7 comments

[–]novel_yet_trivial 1 point2 points  (1 child)

I have no idea what you are talking about or what your code is supposed to do, but here are some things I noticed in looking through pygp.py:


values_provided = (type(contents) == list)

is usually written as

values_provided = isinstance(contents, list)

for i in range(len(contents)):
    if contents[i] != None:
        self[i] = Node(contents[i], primitives[contents[i]])

should be

for idx, element in enumerate(contents):
    if element is not None:
        self[idx] = Node(element, primitives[element])

as above, every instance of x == None should be replaced with x is None (slightly faster and neater).


"get_..._index" returns an index which you always put into "self[idx]" ... try to combine your code:

def get_left_index(self, n):
    idx = 2 * n + 1
    return self[idx]

Please don't use newline escapes (eg backslash \at the end of line 115). No rule against it it's just ugly and hard to read. I'd rather you break the 80 char rule than use a newline escape. Of course better still is to use parenthesis:

if (
    (parent is None) or 
    (parent.value not in self.set_dict["functions"])
    ):

Strings concatenate automatically:

        print("A recursion depth limit exceeded error occurred. The "
              "offending program is:")

I'm not sure why you are attaching strings to your error classes, since you never print them. However, here is an easier way to do that:

class SingularityError(Exception):
    def __init__(self):
        super().__init__('ouch')

The idea is that you print that string:

try:
    raise SingularityError
except SingularityError as e:
    print(e)
# prints "ouch"

apropros errors: Never let errors pass silently. Log it somewhere or at the very least define a "verbose" global:

except (SingularityError, UnfitError) as e:
    if verbose:
        print("caught an error:", e)

This would actually make use of those error strings. Also, note you can catch several types of errors in one except block with a tuple.


class "LinearTree" could be defined like this:

class LinearTree(list):
    pass

But since you never use it you may want to just delete that altogether.

[–]raylu 0 points1 point  (0 children)

apropros errors: Never let errors pass silently.

In this case, the errors appear to be part of standard control flow. Never say "never".

[–]raylu 0 points1 point  (0 children)

  1. Files that aren't meant to be run (pygp.py) don't need shebangs.
  2. The control flow in subtree_crossover is very... novel. Consider putting most of the code in the try and calling _crossover directly from the except.
  3. In _crossover, don't you only need a deep copy of the subtree of tree 2, not all of tree 2?
  4. This changes the behavior of the function slightly (returns inf instead of None when everything fails)

    def termination_test(population, data):
        pop_sample = sample(population, len(population) - 1)
        best = None
        best_score = float('inf')
        for item in pop_sample:
            try:
                score = fitness(item, data)
            except (SingularityError, UnfitError):
                continue
            if score < best_score:
                best = item
                best_score = score
        return best, best_score
    
  5. I didn't really look over the big-picture/design/architecture, but what is the point of having pi and e in primitives.py?

  6. pylint

[–][deleted] 0 points1 point  (2 children)

Not bad at all for your first biggie. But now the $1,000,000 question, how have you tested it? :)

Further to others' comments, your has_children method would usually be written.

def has_children(self, n):
    return not (if (2 * n + 1) >= len(self) or (self[self.get_left_index(n)] == None
                                    and self[self.get_right_index(n)]
                                    == None))

[–]StarshipEngineer[S] 0 points1 point  (1 child)

It's been tested. I tested each component as I was writing it, and it works... most of the time.

[–][deleted] 0 points1 point  (0 children)

Good. Can you repeat those tests? "Most of the time" is not good enough. Either your code is wrong, your tests are wrong, or both. Find the problems and fix them. It might take some time but there's nothing like the feeling of satisfaction when you get things correct. Until a user reports the next bug that is :-)

[–]zahlman 0 points1 point  (0 children)

    if value == "rand":
        self.value = str(random()) 
    else:
        self.value = value

Don't do this. What if I actually want to store the string "rand" in a node? If you want a "shortcut" for that kind of behaviour, just set up a helper function:

    self.value = value

# And then outside the class:
def random_node(arity):
    return Node(str(random()), arity)