Skip to content

Conversation

@tafsiri
Copy link
Contributor

@tafsiri tafsiri commented Jun 17, 2020

To see the logs from the Cloud Build CI, please join either our discussion or announcement mailing list.


This change is Reviewable

Copy link
Collaborator

@lina128 lina128 left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 1 approvals obtained (waiting on @annxingyuan and @tafsiri)


tfjs-core/src/ops/band_part.ts, line 71 at r1 (raw file):

function bandPart_<T extends Tensor>(
    a: T|TensorLike, numLower: number, numUpper: number): T {
  if (numLower % 1 !== 0) {

Use util.assert?


tfjs-core/src/ops/band_part.ts, line 86 at r1 (raw file):

  }

  const shape = $a.shape, [M, N] = $a.shape.slice(-2);

One variable per declaration. https://google.github.io/styleguide/jsguide.html#features-one-variable-per-declaration


tfjs-core/src/ops/band_part.ts, line 106 at r1 (raw file):

  }

  const i = range(0, M, 1, 'int32').reshape([-1, 1]),

One variable per declaration.


tfjs-core/src/ops/band_part.ts, line 106 at r1 (raw file):

  }

  const i = range(0, M, 1, 'int32').reshape([-1, 1]),

Remove chained op.


tfjs-core/src/ops/band_part.ts, line 115 at r1 (raw file):

  const zero = zeros([M, N], $a.dtype);

  return stack(unstack($a.reshape([-1, M, N]))

Remove chained op.


tfjs-core/src/ops/band_part.ts, line 117 at r1 (raw file):

  return stack(unstack($a.reshape([-1, M, N]))
                   .map(mat => where(inBand, mat, zero)))
             .reshape(shape) as T;

Remove chained op.


tfjs-core/src/ops/gram_schmidt.ts, line 91 at r1 (raw file):

      if (i > 0) {
        for (let j = 0; j < i; ++j) {
          const proj = sum(ys[j].mul(x)).mul(ys[j]);

Remove chained op.


tfjs-core/src/ops/gram_schmidt.ts, line 92 at r1 (raw file):

        for (let j = 0; j < i; ++j) {
          const proj = sum(ys[j].mul(x)).mul(ys[j]);
          x = x.sub(proj);

Remove chained op.


tfjs-core/src/ops/gram_schmidt.ts, line 95 at r1 (raw file):

        }
      }
      return x.div(norm(x, 'euclidean'));

Remove chained op.


tfjs-core/src/ops/qr.ts, line 70 at r1 (raw file):

 */
function qr_(x: Tensor, fullMatrices = false): [Tensor, Tensor] {
  if (x.rank < 2) {

util.assert


tfjs-core/src/ops/qr.ts, line 81 at r1 (raw file):

    //   perform QR decomposition on them and then stack the results back
    //   together. We should explore whether this can be parallelized.
    const outerDimsProd = x.shape.slice(0, x.shape.length - 2)

Remove chained op.


tfjs-core/src/ops/qr.ts, line 84 at r1 (raw file):

                              .reduce((value, prev) => value * prev);
    const x2ds = unstack(
        x.reshape([

Removed chained op.


tfjs-core/src/ops/qr.ts, line 96 at r1 (raw file):

      r2ds.push(r2d);
    });
    const q = stack(q2ds, 0).reshape(x.shape);

Remove chained op.


tfjs-core/src/ops/qr.ts, line 97 at r1 (raw file):

    });
    const q = stack(q2ds, 0).reshape(x.shape);
    const r = stack(r2ds, 0).reshape(x.shape);

Remove chained op.


tfjs-core/src/ops/qr.ts, line 104 at r1 (raw file):

function qr2d(x: Tensor2D, fullMatrices = false): [Tensor2D, Tensor2D] {
  return ENGINE.tidy(() => {
    if (x.shape.length !== 2) {

util.assert


tfjs-core/src/ops/qr.ts, line 113 at r1 (raw file):

    let q = eye(m);     // Orthogonal transform so far.
    let r = x.clone();  // Transformed matrix so far.

Remove chained op.


tfjs-core/src/ops/qr.ts, line 116 at r1 (raw file):

    const one2D = tensor2d([[1]], [1, 1]);
    let w: Tensor2D = one2D.clone();

Remove chained op.


tfjs-core/src/ops/qr.ts, line 127 at r1 (raw file):

      [w, r, q] = ENGINE.tidy((): [Tensor2D, Tensor2D, Tensor2D] => {
        // Find H = I - tau * w * w', to put zeros below R(j, j).
        const rjEnd1 = r.slice([j, j], [m - j, 1]);

Remove chained ops here and below.

@lina128 lina128 self-requested a review June 17, 2020 03:27
Copy link
Contributor Author

@tafsiri tafsiri left a comment

Choose a reason for hiding this comment

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

Thanks Na!

Reviewable status: 0 of 1 approvals obtained (waiting on @annxingyuan and @lina128)


tfjs-core/src/ops/band_part.ts, line 71 at r1 (raw file):

Previously, lina128 (Na Li) wrote…

Use util.assert?

Done. Here and below.


tfjs-core/src/ops/band_part.ts, line 86 at r1 (raw file):

Previously, lina128 (Na Li) wrote…

One variable per declaration. https://google.github.io/styleguide/jsguide.html#features-one-variable-per-declaration

Done


tfjs-core/src/ops/band_part.ts, line 106 at r1 (raw file):

Previously, lina128 (Na Li) wrote…

One variable per declaration.

Done


tfjs-core/src/ops/band_part.ts, line 106 at r1 (raw file):

Previously, lina128 (Na Li) wrote…

Remove chained op.

Done


tfjs-core/src/ops/band_part.ts, line 115 at r1 (raw file):

Previously, lina128 (Na Li) wrote…

Remove chained op.

Done


tfjs-core/src/ops/band_part.ts, line 117 at r1 (raw file):

Previously, lina128 (Na Li) wrote…

Remove chained op.

Done


tfjs-core/src/ops/gram_schmidt.ts, line 91 at r1 (raw file):

Previously, lina128 (Na Li) wrote…

Remove chained op.

Done


tfjs-core/src/ops/gram_schmidt.ts, line 92 at r1 (raw file):

Previously, lina128 (Na Li) wrote…

Remove chained op.

Done


tfjs-core/src/ops/gram_schmidt.ts, line 95 at r1 (raw file):

Previously, lina128 (Na Li) wrote…

Remove chained op.

Done


tfjs-core/src/ops/qr.ts, line 70 at r1 (raw file):

Previously, lina128 (Na Li) wrote…

util.assert

Done


tfjs-core/src/ops/qr.ts, line 81 at r1 (raw file):

Previously, lina128 (Na Li) wrote…

Remove chained op.

in this case these are javascript array functions (on shape)


tfjs-core/src/ops/qr.ts, line 84 at r1 (raw file):

Previously, lina128 (Na Li) wrote…

Removed chained op.

Done


tfjs-core/src/ops/qr.ts, line 96 at r1 (raw file):

Previously, lina128 (Na Li) wrote…

Remove chained op.

Done


tfjs-core/src/ops/qr.ts, line 97 at r1 (raw file):

Previously, lina128 (Na Li) wrote…

Remove chained op.

Done


tfjs-core/src/ops/qr.ts, line 104 at r1 (raw file):

Previously, lina128 (Na Li) wrote…

util.assert

Done


tfjs-core/src/ops/qr.ts, line 113 at r1 (raw file):

Previously, lina128 (Na Li) wrote…

Remove chained op.

Done


tfjs-core/src/ops/qr.ts, line 116 at r1 (raw file):

Previously, lina128 (Na Li) wrote…

Remove chained op.

Done


tfjs-core/src/ops/qr.ts, line 127 at r1 (raw file):

Previously, lina128 (Na Li) wrote…

Remove chained ops here and below.

Done

Copy link
Collaborator

@lina128 lina128 left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 11 files at r1.
Reviewable status: :shipit: complete! 1 of 1 approvals obtained (waiting on @annxingyuan and @lina128)

@tafsiri tafsiri merged commit d23be0c into master Jun 17, 2020
@tafsiri tafsiri deleted the mod-linalg branch June 17, 2020 18:07
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.

3 participants