Skip to content

[C++ API Parity] - TripletMarginLoss #27713

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.

Sign up for GitHub

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

Jump to bottom
Closed
PyExtreme wants to merge 5 commits into from
Closed

[C++ API Parity] - TripletMarginLoss #27713

PyExtreme wants to merge 5 commits into from

Conversation

@PyExtreme
Copy link

@PyExtreme PyExtreme commented Oct 10, 2019

Hi @yf225 , I had to create a new branch to tackle merge conflict since I am using cloud due to some limitations on my PC. Therefore, I don't have enough command there.

Also, I have incorporated the changes you have put before here
#27613

Also, it would be great if you could recommend me some resources to work smmothly on GCP..:-D

Thank you

@pytorchbot pytorchbot added the module: cpp Related to C++ API label Oct 10, 2019
@PyExtreme PyExtreme mentioned this pull request Oct 10, 2019
Closed
yf225
yf225 reviewed Oct 10, 2019
View reviewed changes
torch/csrc/api/src/nn/modules/loss.cpp Outdated
Copy link
Contributor

@yf225 yf225 Oct 10, 2019

Choose a reason for hiding this comment

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

We are missing a / here:

Suggested change
/ ============================================================================
// ============================================================================

@yf225
Copy link
Contributor

yf225 commented Oct 10, 2019

@PyExtreme curious what's your current workflow on GCP? Are you able to run git pull / git push on it?

@PyExtreme
Copy link
Author

PyExtreme commented Oct 11, 2019 •
edited
Loading

@yf225 , I have made the change.
I am sorry for these silly mistakes which could have been avoided on careful review.

So there are two problems, I face while using GCP.

  1. I have to use vim/nano for writing code. I am now gonna use codeserver for this but the pre-release which is compatible with the vscode version, seems like does not have a password based mechanism to prevent someone else accessing my IP.

  2. Due to above mentioned text editors, merging becomes a bit tough. Are there any good text editors or a source-code editor like VSCode which can help me fix conflicts easily while working on cloud(through some remote server)?

Thank you so much for your time. I have gained a lot from here.

@PyExtreme
Copy link
Author

PyExtreme commented Oct 13, 2019

@yf225 , could you please have a look at it. I have made all the necessary changes.

yf225
yf225 approved these changes Oct 13, 2019
View reviewed changes
Copy link
Contributor

@yf225 yf225 left a comment

Choose a reason for hiding this comment

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

@PyExtreme Sorry for the delay and thanks so much for the awesome work! For fixing the code conflicts, I think as long as we are able to edit the source code we should be able to find all the <<<<< occurrences and then fix them. I usually just use vim / Sublime Text for it :D

facebook-github-bot
facebook-github-bot reviewed Oct 13, 2019
View reviewed changes
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@yf225 is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Oct 13, 2019

@yf225 merged this pull request in 52528c0.

@yf225 yf225 mentioned this pull request Oct 14, 2019
Open
This was referenced Nov 17, 2019
Closed
Closed
thiagocrepaldi pushed a commit to thiagocrepaldi/pytorch that referenced this pull request Feb 4, 2020
@jhaankit373
- TripletMarginLoss (pytorch#27713)
08c65ec 
Summary:
Hi yf225 , I had to create a new branch to tackle merge conflict since I am using cloud due to some limitations on my PC. Therefore, I don't have enough command there.

Also, I have incorporated the changes you have put before here
pytorch#27613

Also, it would be great if you could recommend me some resources to work smmothly on GCP..:-D

Thank you
Pull Request resolved: pytorch#27713

Differential Revision: D17899695

Pulled By: yf225

fbshipit-source-id: eb6643223148774a5cbbd093bdcc5623872e5bba
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@facebook-github-bot facebook-github-bot facebook-github-bot left review comments

@yf225 yf225 yf225 approved these changes

@ebetica ebetica Awaiting requested review from ebetica

@goldsborough goldsborough Awaiting requested review from goldsborough

Assignees

No one assigned

Labels

Merged module: cpp Related to C++ API

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@PyExtreme @yf225 @facebook-github-bot @pytorchbot @mruberry @jhaankit373