search

PIA-Group / BioSPPy

Biosignal Processing in Python
Other
569 stars 273 forks source link

Confusion of implementation of utils.random_fraction #60

Closed ilongshan closed 3 years ago

ilongshan commented 5 years ago

In utils.random_fraction, I find the following code,

   # copy because shuffle works in place
    aux = copy.deepcopy(indx)

    # shuffle
    np.random.shuffle(indx)

    # select
    use = aux[:nb]
    unuse = aux[nb:]

About the shuffle, should it shuffle aux instead of index?

afonsocraposo commented 3 years ago

Closed due to inactivity. Feel free to reopen.

ilongshan commented 3 years ago

@Afonsocraposo utils.random_fraction is used to select a random fraction of an input list of elements. The input list indx first is deepcopy to aux. And then use = aux[:nb] and unuse = aux[nb:] are returned. Since aux is not shuffled, the returned use and unused are not random partition of the input list indx. So

# shuffle
    np.random.shuffle(indx)

should be

# shuffle
    np.random.shuffle(aux)

to get random partition?

afonsocraposo commented 3 years ago

Thank you so much for the explanation @ilongshan You're right! This issue was fixed with commit https://github.com/PIA-Group/BioSPPy/commit/52340610f850f382082136cd645496e22fbdbae5.

ilongshan commented 3 years ago

@Afonsocraposo Great!. Thanks for the quick response.