Skip to content

Conversation

@qingqing01
Copy link
Collaborator

@qingqing01 qingqing01 commented Jun 7, 2017

Fix #75

Copy link
Contributor

@xinghai-sun xinghai-sun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! Almost LGTM.

:type manifest: list
:param batch_size: batch size.
:type batch_size: int
"""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a :return: and :rtype:

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

:param sortagrad: Sort the audio clips by duration in the first epoc
if set True (for SortaGrad).
:type sortagrad: bool
:param shuffle: Shuffle the audio clips if set True.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it is not a thorough instance-wise shuffle, but a specific batch-wise shuffle. Could you add more explanation to users?
Besides, rename shuffle to batch_shuffle?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

在__batch_shuffle__里增加了更多注释。

def instance_reader_creator(self,
manifest_path,
sort_by_duration=True,
batch_size,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here batch_size is for batch_shuffle,

  1. put batch_size just before shuffle
  2. rename batch_size --> batch_shuffle_size (否则有点奇怪,因为函数名是instance reader,用户不知道为什么和batch_size有关?)
    for a better understanding?
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

有道理。我把代码做了调整,instance_reader_creator()只保留如何读取instance。

sort_by_duration=True,
batch_size,
sortagrad=True,
shuffle=False):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shuffle-->batch_shuffle ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

去掉了该参数~

flatten=False,
sort_by_duration=True,
sortagrad=False,
shuffle=False):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shuffle --> batch_shuffle

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

manifest_path,
sort_by_duration=True,
shuffle=False):
def __batch_shuffle__(self, manifest, batch_size):
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里确实需要batch_size,因此没有改成batch_shuffle_size

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants