Skip to content

Conversation

@zou3519
Copy link
Contributor

@zou3519 zou3519 commented Nov 4, 2019

Stack from ghstack:

cdist(x1, x2) does the following:

  • assume x1, x2 are 2-dimensional. Then x1, x2 are each considered to be
    a list of vectors.
  • The operation returns a matrix that is the pairwise distance between
    each vector in x1 and each vector in x2. The matrix has first dimension
    size equal to the number of vectors in x1 and second dimension size equal
    to the number of vectors in x2.
  • cdist also supports arbitrary left-hand broadcastable batch
    dimensions. In this case, x1 and x2 are each considered to be a batch
    of a list of vectors.

The above leads to the following name inference rule for cdist:

  • In the 2D case, propagate x1.names[-2] and x2.names[-1] (because
    the final result has size (x1.size[-2], x2.size[-2]).
  • in the ND case, unify all the batch dimensions together to produce the
    output batch dimensions and then apply the rule for the 2D case.

Furthermore, I moved all of the name checking in the implementation to
occur before name inference because name inference assumes that the
shapes are valid.

Test Plan:

  • new test: pytest test/test_namedtensor.py -v -k "cdist"

Differential Revision: D18311867

cdist(x1, x2) does the following:
- assume x1, x2 are 2-dimensional. Then x1, x2 are each considered to be
a list of vectors.
- The operation returns a matrix that is the pairwise distance between
each vector in x1 and each vector in x2. The matrix has first dimension
size equal to the number of vectors in x1 and second dimension size equal
to the number of vectors in x2.
- cdist also supports arbitrary left-hand broadcastable batch
dimensions. In this case, x1 and x2 are each considered to be a batch
of a list of vectors.

The above leads to the following name inference rule for cdist:
- In the 2D case, propagate x1.names[-2] and x2.names[-1] (because
the final result has size (x1.size[-2], x2.size[-2]).
- in the ND case, unify all the batch dimensions together to produce the
output batch dimensions and then apply the rule for the 2D case.

Furthermore, I moved all of the name checking in the implementation to
occur before name inference because name inference assumes that the
shapes are valid.

Test Plan:
- new test: `pytest test/test_namedtensor.py -v -k "cdist"`

[ghstack-poisoned]
cdist(x1, x2) does the following:
- assume x1, x2 are 2-dimensional. Then x1, x2 are each considered to be
a list of vectors.
- The operation returns a matrix that is the pairwise distance between
each vector in x1 and each vector in x2. The matrix has first dimension
size equal to the number of vectors in x1 and second dimension size equal
to the number of vectors in x2.
- cdist also supports arbitrary left-hand broadcastable batch
dimensions. In this case, x1 and x2 are each considered to be a batch
of a list of vectors.

The above leads to the following name inference rule for cdist:
- In the 2D case, propagate x1.names[-2] and x2.names[-1] (because
the final result has size (x1.size[-2], x2.size[-2]).
- in the ND case, unify all the batch dimensions together to produce the
output batch dimensions and then apply the rule for the 2D case.

Furthermore, I moved all of the name checking in the implementation to
occur before name inference because name inference assumes that the
shapes are valid.

Test Plan:
- new test: `pytest test/test_namedtensor.py -v -k "cdist"`

[ghstack-poisoned]
cdist(x1, x2) does the following:
- assume x1, x2 are 2-dimensional. Then x1, x2 are each considered to be
a list of vectors.
- The operation returns a matrix that is the pairwise distance between
each vector in x1 and each vector in x2. The matrix has first dimension
size equal to the number of vectors in x1 and second dimension size equal
to the number of vectors in x2.
- cdist also supports arbitrary left-hand broadcastable batch
dimensions. In this case, x1 and x2 are each considered to be a batch
of a list of vectors.

The above leads to the following name inference rule for cdist:
- In the 2D case, propagate x1.names[-2] and x2.names[-1] (because
the final result has size (x1.size[-2], x2.size[-2]).
- in the ND case, unify all the batch dimensions together to produce the
output batch dimensions and then apply the rule for the 2D case.

Furthermore, I moved all of the name checking in the implementation to
occur before name inference because name inference assumes that the
shapes are valid.

Test Plan:
- new test: `pytest test/test_namedtensor.py -v -k "cdist"`

[ghstack-poisoned]
zou3519 added a commit that referenced this pull request Nov 4, 2019
cdist(x1, x2) does the following:
- assume x1, x2 are 2-dimensional. Then x1, x2 are each considered to be
a list of vectors.
- The operation returns a matrix that is the pairwise distance between
each vector in x1 and each vector in x2. The matrix has first dimension
size equal to the number of vectors in x1 and second dimension size equal
to the number of vectors in x2.
- cdist also supports arbitrary left-hand broadcastable batch
dimensions. In this case, x1 and x2 are each considered to be a batch
of a list of vectors.

The above leads to the following name inference rule for cdist:
- In the 2D case, propagate x1.names[-2] and x2.names[-1] (because
the final result has size (x1.size[-2], x2.size[-2]).
- in the ND case, unify all the batch dimensions together to produce the
output batch dimensions and then apply the rule for the 2D case.

Furthermore, I moved all of the name checking in the implementation to
occur before name inference because name inference assumes that the
shapes are valid.

Test Plan:
- new test: `pytest test/test_namedtensor.py -v -k "cdist"`

ghstack-source-id: dbf7aeb
Pull Request resolved: #29129
@zou3519 zou3519 requested review from gchanan, izdeby and nairbv November 4, 2019 18:00

auto result = self_batch.unifyFromRight(other_batch, "cdist");
result.append(TensorName(self_names, -2));
result.append(TensorName(other_names, -2));
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you, please, leave a in-place comment that will explain '-2' value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do

cdist(x1, x2) does the following:
- assume x1, x2 are 2-dimensional. Then x1, x2 are each considered to be
a list of vectors.
- The operation returns a matrix that is the pairwise distance between
each vector in x1 and each vector in x2. The matrix has first dimension
size equal to the number of vectors in x1 and second dimension size equal
to the number of vectors in x2.
- cdist also supports arbitrary left-hand broadcastable batch
dimensions. In this case, x1 and x2 are each considered to be a batch
of a list of vectors.

The above leads to the following name inference rule for cdist:
- In the 2D case, propagate x1.names[-2] and x2.names[-1] (because
the final result has size (x1.size[-2], x2.size[-2]).
- in the ND case, unify all the batch dimensions together to produce the
output batch dimensions and then apply the rule for the 2D case.

Furthermore, I moved all of the name checking in the implementation to
occur before name inference because name inference assumes that the
shapes are valid.

Test Plan:
- new test: `pytest test/test_namedtensor.py -v -k "cdist"`

Differential Revision: [D18311867](https://our.internmc.facebook.com/intern/diff/D18311867)

[ghstack-poisoned]
zou3519 added a commit that referenced this pull request Nov 4, 2019
cdist(x1, x2) does the following:
- assume x1, x2 are 2-dimensional. Then x1, x2 are each considered to be
a list of vectors.
- The operation returns a matrix that is the pairwise distance between
each vector in x1 and each vector in x2. The matrix has first dimension
size equal to the number of vectors in x1 and second dimension size equal
to the number of vectors in x2.
- cdist also supports arbitrary left-hand broadcastable batch
dimensions. In this case, x1 and x2 are each considered to be a batch
of a list of vectors.

The above leads to the following name inference rule for cdist:
- In the 2D case, propagate x1.names[-2] and x2.names[-1] (because
the final result has size (x1.size[-2], x2.size[-2]).
- in the ND case, unify all the batch dimensions together to produce the
output batch dimensions and then apply the rule for the 2D case.

Furthermore, I moved all of the name checking in the implementation to
occur before name inference because name inference assumes that the
shapes are valid.

Test Plan:
- new test: `pytest test/test_namedtensor.py -v -k "cdist"`

ghstack-source-id: d5fb2be
Pull Request resolved: #29129
zdevito pushed a commit to zdevito/ATen that referenced this pull request Nov 5, 2019
Summary:
Pull Request resolved: pytorch/pytorch#29129

cdist(x1, x2) does the following:
- assume x1, x2 are 2-dimensional. Then x1, x2 are each considered to be
a list of vectors.
- The operation returns a matrix that is the pairwise distance between
each vector in x1 and each vector in x2. The matrix has first dimension
size equal to the number of vectors in x1 and second dimension size equal
to the number of vectors in x2.
- cdist also supports arbitrary left-hand broadcastable batch
dimensions. In this case, x1 and x2 are each considered to be a batch
of a list of vectors.

The above leads to the following name inference rule for cdist:
- In the 2D case, propagate x1.names[-2] and x2.names[-1] (because
the final result has size (x1.size[-2], x2.size[-2]).
- in the ND case, unify all the batch dimensions together to produce the
output batch dimensions and then apply the rule for the 2D case.

Furthermore, I moved all of the name checking in the implementation to
occur before name inference because name inference assumes that the
shapes are valid.

Test Plan: - new test: `pytest test/test_namedtensor.py -v -k "cdist"`

Differential Revision: D18311867

Pulled By: zou3519

fbshipit-source-id: 713d7cdda93c8fe92e7f1bd7f7c5c6e20a8138e3
@facebook-github-bot
Copy link
Contributor

@zou3519 merged this pull request in cb6d9de.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants