Skip to content

Adding SubtractOperation and AverageOperation#25

Open
uditarora wants to merge 3 commits intodevelopfrom
feature/more-operations
Open

Adding SubtractOperation and AverageOperation#25
uditarora wants to merge 3 commits intodevelopfrom
feature/more-operations

Conversation

@uditarora
Copy link
Collaborator

  • SubtractOperation allows subtraction between two tensors and enables gradient to flow through the subtraction
  • AverageOperation allows taking the average of a vector of tensors and enables gradient to flow through the average

The average operation will be useful in computing losses across a batch of data.

@uditarora uditarora requested a review from karanchahal April 11, 2019 09:36
Copy link
Owner

@karanchahal karanchahal left a comment

Choose a reason for hiding this comment

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

Prelim Review.

class AverageOperation : public Operation<T> {
public:

vector<Tensor<T*>> tensors;
Copy link
Owner

Choose a reason for hiding this comment

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

Thanks for the pull request!! This is a prelim review.

The representation of the Average Tensor will need to be changed I think. Generally how the network takes in a tensor is a single tensor with size [batch_size, input_dim]. SO the average operation should take as input t1 which would be a tensor and another int variable called axis specifying what dimension to average along.

Hence the API would look like

loss = losses::mse(y,gt)
averageLoss = tensorOps::average(loss,axis=0)
// and then continue from here

This would be inline how pytorch, tf and numpy handle some operation along a list of objects. Hence, I think it would be better to adhere with their API.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right, I missed the big picture here. Thanks for pointing this out.

Copy link
Owner

Choose a reason for hiding this comment

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

No problem, looking forward to your updates :)

#ifndef __OP_SUBTRACT_IMPL_INCLUDED__
#define __OP_SUBTRACT_IMPL_INCLUDED__

template <typename T>
Copy link
Owner

Choose a reason for hiding this comment

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

Comments on top of the function. Can remove comments inside function

// Average
template<typename T>
Tensor<T>* average(vector<Tensor<T>*>& tensors) {
if (tensors.size() == 0) { // To avoid division by zero. Should we do this?
Copy link
Owner

Choose a reason for hiding this comment

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

Don't think this check will be needed if we adhere to the requested API

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.

2 participants