Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add doc for AggregateLevel and ExpandLevel #2200

Merged
merged 2 commits into from
May 18, 2017
Merged

add doc for AggregateLevel and ExpandLevel #2200

merged 2 commits into from
May 18, 2017

Conversation

luotao1
Copy link
Contributor

@luotao1 luotao1 commented May 18, 2017

  1. fix Documentation for AggregateLevel #2093
  2. add documentation of ExpandLevel
  3. as AggregateLevel and ExpandLevel are in conf_helps (paddle.trainer_config_helpers), change conf_helps.layers.AggregateLevel to conf_helps.AggregateLevel.

Copy link
Contributor

@lcy-seso lcy-seso left a comment

Choose a reason for hiding this comment

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

some small comments.

- :code:`SequenceType.NO_SEQUENCE` means the sample is not a sequence.
- :code:`SequenceType.SEQUENCE` means the sample is a sequence.
- :code:`SequenceType.SUB_SEQUENCE` means it is a nested sequence, that
each timestep of the input sequence is also a sequence.
Copy link
Contributor

Choose a reason for hiding this comment

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

SequenceType.SUB_SEQUENCE means the sample is a nested sequence, each timestep of which is also a sequence.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

- :code:`SequenceType.SUB_SEQUENCE` means it is a nested sequence, that
each timestep of the input sequence is also a sequence.

Thus, AggregateLevel supports two modes:
Copy link
Contributor

Choose a reason for hiding this comment

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

Thus --> Accordingly,
我的理解是:支持三种序列 --> 针对这三种序列类型,对应有两种聚合方式。
而 Thus 是一种强因果关系。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@@ -225,6 +225,24 @@ def is_layer_type(type_name):


class AggregateLevel(object):
"""
As PaddlePaddle supports three sequence types:
Copy link
Contributor

Choose a reason for hiding this comment

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

As PaddlePaddle supports three sequence types --> PaddlePaddle supports three sequence types

去掉 As

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Thus, AggregateLevel supports two modes:

- :code:`AggregateLevel.EACH_TIMESTEP` means the aggregation acts on each
timestep of sequence, both :code:`SUB_SEQUENCE` and :code:`SEQUENCE` will
Copy link
Contributor

Choose a reason for hiding this comment

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

means the aggregation acts on each timestep of sequence --> means the aggregation acts on each timestep of a sequence

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

@lcy-seso lcy-seso left a comment

Choose a reason for hiding this comment

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

LGTM

@lcy-seso lcy-seso merged commit 7377c59 into PaddlePaddle:develop May 18, 2017
Accordingly, AggregateLevel supports two modes:

- :code:`AggregateLevel.EACH_TIMESTEP` means the aggregation acts on each
timestep of a sequence, both :code:`SUB_SEQUENCE` and :code:`SEQUENCE` will
Copy link
Collaborator

Choose a reason for hiding this comment

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

This actually aggregates over the whole sequence. Maybe change the name to TO_NON_SEUQNCE will be easier to understand.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it, I will change the name in next PR.


ExpandLevel supports two modes:

- :code:`ExpandLevel.FROM_TIMESTEP` means the expandation acts on each
Copy link
Collaborator

Choose a reason for hiding this comment

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

expandation => expansion

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it, I will fix it in next PR.

timestep of a sequence, :code:`NO_SEQUENCE` will be expanded to
:code:`SEQUENCE` or :code:`SUB_SEQUENCE`.

- :code:`ExpandLevel.FROM_SEQUENCE` means the expandation acts on each
Copy link
Collaborator

Choose a reason for hiding this comment

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

expandation=> expansion

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it, I will fix it in next PR.


- :code:`ExpandLevel.FROM_TIMESTEP` means the expandation acts on each
timestep of a sequence, :code:`NO_SEQUENCE` will be expanded to
:code:`SEQUENCE` or :code:`SUB_SEQUENCE`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

"acts on each timestep of a sequence" is not correct because the input for FROM_TIMESTEP is non-sequence. Perhaps we should consider change the name to FROM_NON_SEQUENCE.

Copy link
Contributor Author

@luotao1 luotao1 May 19, 2017

Choose a reason for hiding this comment

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

Got it, I will change the name in next PR.

timestep of a sequence, both :code:`SUB_SEQUENCE` and :code:`SEQUENCE` will
be aggregated to :code:`NO_SEQUENCE`.

- :code:`AggregateLevel.EACH_SEQUENCE` means the aggregation acts on each
Copy link
Collaborator

Choose a reason for hiding this comment

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

Accordingly, TO_SEQUENCE will be easier to understand.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it, I will change the name in next PR.

@luotao1 luotao1 deleted the agg_level branch May 19, 2017 02:09
heavengate pushed a commit to heavengate/Paddle that referenced this pull request Aug 16, 2021
* fix bbox_num 0 after nms, test=dygraph

* add copyright, test=dygraph
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Documentation for AggregateLevel
3 participants