Skip to content
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.

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

Integration of TorchSharp and LibTorchSharp: exposing LibTorch C++ training API #80

Merged
merged 113 commits into from
Jun 24, 2019

Conversation

artidoro
Copy link
Contributor

@artidoro artidoro commented Jun 12, 2019

This PR fixes #79.

A summary of what this PR does:

  1. Merges interesaaat/TorchSharp and interesaaat/LibTorchSharp (preserving as much as possible the file history).
  2. Combines native and managed code using the same build system used in ML.NET.
  3. Creates a LibTorch.Redist nuget package which redistributes LibTorch C++ frontend of Pytorch as a nuget package (cleared any legal concerns with CELA).

Shortcomings (things that will need fixing):

  1. Need to point the official build to an actual nuget feed so that it uploads the packages automatically (this could be a DevOps feed or a MyGet feed or a nuget.org feed).
  2. The documentation will need to be fixed. I have just moved it to the docs folder.
  3. There are a few tests from interesaaat/TorchSharp that I am currently skipping and which should be re-enabled.
  4. The LibTorch.Redist pacakge has only the CPU dlls. It should be straightforward at this point to add the GPU dlls, and create a new pacakge LibTorch.GPU.Redist which contains the additional GPU dlls.

Notice:
We probably need to instruct DevOps to look for new build definitions in the following two yml files (one for the official build and the other for the PR verification build).

Let me know if I should target another branch (other than master) to begin, and of course I am happy to make any changes that you think are needed to check this in.

/cc: @interesaaat @MiguelCaldasMS

Added types for Torch tensors.
* Added a way to create pinned array from already allocated arrays
 * naming with functional modules
 * iterator instead of memory copy

Still weill need to figure out how to reset the iterator to implement epochs.
* Fixed MNIST example
@interesaaat
Copy link
Contributor

  1. Need to point the official build to an actual nuget feed so that it uploads the packages automatically (this could be a DevOps feed or a MyGet feed or a nuget.org feed).

I believe Miguel already set up this, we probably only need to point to the right build.

@@ -1,18 +1,17 @@
[![Build Status](https://migueldeicaza.visualstudio.com/TorchSharp/_apis/build/status/TorchSharp-CI)](https://migueldeicaza.visualstudio.com/TorchSharp/_build/latest?definitionId=5)

TorchSharp
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 update the README with the new info on how to build / platform supported?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also we have Azure Pipelines set up, right? (At least for build and test). Can you please add those too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think Miguel needs to add a new pipeline from DevOps since I have changed the file names of the yml files specifying the build instructions.

test/TorchSharpTest/TorchSharp/BaseTestClass.cs Outdated Show resolved Hide resolved
@interesaaat
Copy link
Contributor

To me the PR is ok. We will fill a couple of issues to address the known limitations once merged. @migueldeicaza if is ok with I will go ahead and merge it. We will need your help to set up the pipelines (we don't have access to the DevOps account for this repo) and to refresh the docs.

…t work. Added an example of if statement in a model to test that eager evaluation is actually correct.
@migueldeicaza
Copy link
Contributor

Agreed - let us merge it now and work on the outstanding issues. Would love to know what happened with the docs that needs work

@migueldeicaza migueldeicaza merged commit 4c511cf into dotnet:master Jun 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Merging LibTorchSharp in TorchSharp
4 participants