Skip to content

Conversation

@tafsiri
Copy link
Contributor

@tafsiri tafsiri commented Jun 22, 2020

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


This change is Reviewable

@tafsiri tafsiri requested a review from lina128 June 22, 2020 22:15
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 7 of 24 files at r1.
Reviewable status: :shipit: complete! 1 of 1 approvals obtained (waiting on @lina128 and @tafsiri)


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

 */
/** @doc {heading: 'Training', subheading: 'Losses', namespace: 'losses'} */
function absoluteDifference_<T extends Tensor, O extends Tensor>(

I know this is from the original code, but just curious, why use O instead of just use T? tensorflow/tfjs-core#775


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

Quoted 16 lines of code…
/**
 * @license
 * Copyright 2018 Google Inc. All Rights Reserved.
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 * http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 * =============================================================================
 */

Remove


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

  return computeWeightedLoss(losses, $weights, reduction);
}

nit: Add a newline below.


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

  return computeWeightedLoss(losses, $weights, reduction);
}

nit: Add a newline below.

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.

Reviewable status: :shipit: complete! 1 of 1 approvals obtained (waiting on @lina128)


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

Previously, lina128 (Na Li) wrote…

I know this is from the original code, but just curious, why use O instead of just use T? tensorflow/tfjs-core#775

Good question, I wasn't really sure but my guess looking at the code is that it allows a typescript user to specify the rank of the input and output tensors respectively (since there is generally a reduction being performed).


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

Previously, lina128 (Na Li) wrote…
/**
 * @license
 * Copyright 2018 Google Inc. All Rights Reserved.
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 * http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 * =============================================================================
 */

Remove

Done


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

Previously, lina128 (Na Li) wrote…

nit: Add a newline below.

Done


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

Previously, lina128 (Na Li) wrote…

nit: Add a newline below.

Done

@tafsiri tafsiri merged commit 8ab39b7 into master Jun 24, 2020
@tafsiri tafsiri deleted the mod-loss-functions branch June 24, 2020 02:40
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