haotianteng / Chiron

A basecaller for Oxford Nanopore Technologies' sequencers
Other
122 stars 53 forks source link

Mutable Default Argument in CNN module #22

Closed mbhall88 closed 6 years ago

mbhall88 commented 6 years ago

In function conv_layer the parameter stride on this line has a mutable default argument [1, 1, 1, 1]. Is this intentional?

This article explains why this is a problem with the following example:

def append_to(element, to=[]):
    to.append(element)
    return to

my_list = append_to(12)
print(my_list)
#[12]
my_other_list = append_to(42)
print(my_other_list)
#[12, 42]

A new list is created once when the function is defined, and the same list is used in each successive call. Python’s default arguments are evaluated once when the function is defined, not each time the function is called (like it is in say, Ruby). This means that if you use a mutable default argument and mutate it, you will and have mutated that object for all future calls to the function as well.

If this is the functionality you want (i.e continual adding elements into that initial list) then that's fine, we can leave it as is.
But if you want the list to be redefined everytime the function is called, I would suggest changing it to this:

def conv_layer(indata, ksize, padding, training, name, dilate=1,
               strides=None, bias_term=False, active=True,
               BN=True, active_function='relu'):
    if strides is None:
        strides = [1, 1, 1, 1]