Skip to content

[ML] Merge the feature/pytorch-inference branch #1902

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

Merged
merged 37 commits into from
Jun 1, 2021

Conversation

davidkyle
Copy link
Member

@davidkyle davidkyle commented May 24, 2021

Merge the feature/pytorch-inference branch into master.

  • Create a new pytorch_inference binary for evaluating PyTorch Models
  • Add the evaluate.py script for quickly evaluating models with pytorch_inference
  • Document LibTorch build instructions to the setup files (multiple PRs)
  • Build LibTorch for each platform and update the docker build images (multiple PRs)

davidkyle and others added 30 commits January 12, 2021 18:11
The application connects to 4 named pipes and accepts a TorchScript model on
the 'restore' pipe. Input in a series of tokens from the BERT vocabulary and 
the output an JSON document containing the results tensor. Logging is written
to the 4th pipe.

The script evaluate.py will start the app, connect the named pipes and run
an example from the examples directory. This is the easiest way to test and 
develop the app serving as a proxy for the Java/ES side until that is complete.

The app will not be built by a call to the top level makefile so the dependencies 
do not have to be installed yet.
1. Use vanilla Python instead of Anaconda
2. Don't build all the possible libraries, as we
   don't need them (there may be more to do here
   in the future too)
3. Move instructions to main build instructions now
   we are on the road to incorporating this into
   core product
- Adds build setup instructions for PyTorch 1.7.1 on Windows.
- Some minor tweaks to the build system to account for the
  way Windows imports DLLs using import libraries, and the
  extra dependencies of the PyTorch DLLs on Windows.
- WinSock2 is needed for ntohl on Windows.
- Reinstate the --version functionality to the new program.
Builds and installs Python 3.7.9 and CMake then clones
the PyTorch repo and builds libtorch
This change sets extra flags during the PyTorch build to
avoid building unnecessary components, which dramatically
cuts the build time.  It also changes the location of
debug symbols from the libraries themselves to separate
.pdb files.

A couple of tweaks to the 1.7.1 build files are required
to get these options to work.  One of these is already
in the PyTorch master branch code.  There may be time to
get the other one incorporated in time for 1.8.0 too.
1. The IO manager needs to open streams with the binary flag,
   otherwise the stream will stop reading binary files that
   contain an EOF character prematurely.
2. Upload the Windows build tools to S3 so CI can download
   them.
3. Adjust the test script so it uses regular files instead
   of named pipes.  Named pipes are completely different on
   Windows and hard to use from Python.  It's easier to make
   a Python script portable if it uses regular files.
Linux uses the Eigen BLAS library and macOS the Accelerate framework 
so MKL is not required. Delete the pytorch repo once built in the docker
image
FBGEMM requires AVX2 instructions and cannot be used on ARM, Caffe2 is used instead
which is different to the x64 builds.

The linux docker files are changed to use multi-stage builds so the final image does
not contain intermediate dependencies uses to build the actual dependencies.
Now we have built libraries for all the different platforms
pytorch_inference can be added to the Makefile without
breaking CI.

If CI fails for this PR it will smoke out any mistakes in
Docker images or dependency bundles.

Also adding to controller as that's a one-liner.
Delete temp files created by evaluate.py
Defines the input and output format for the PyTorch 3rd party model app
and adds a command processor which parses JSON documents from a 
input stream then calls a handler function for each request.
Upgrades PyTorch from version 1.7.1 to version 1.8.0 on macOS.

It is now possible to build a version that works on Apple Silicon.

The build instructions are also adjusted to remove functions that
call an external compiler to build custom extensions. Although
these would never have worked in our programs due to system call
filtering, it's best if they aren't present at all as they could
alarm heuristic virus scanners.

(Similar upgrades will be done for Windows and Linux in the near
future.)
Upgrades PyTorch from version 1.7.1 to version 1.8.0 on Windows.

The build instructions are also adjusted to remove functions that
call an external compiler to build custom extensions. Although
these would never have worked in our programs due to system call
filtering, it's best if they aren't present at all as they could
alarm heuristic virus scanners.

(A similar upgrade will be done for Linux in the near future.)
Renames pytorch process logging arguments to be
consistent with the other processes and with what java
provides.
The build instructions are also adjusted to remove functions that
call an external compiler to build custom extensions. Although
these would never have worked in our programs due to system call
filtering, it's best if they aren't present at all as they could
alarm heuristic virus scanners.
The details are for version 1.8.0.

PyTorch will also show up in the public dependency report
once the feature branch is merged to master.
This adds an error handler to the command processor that is called
 in the event of a bad inference request. The error is returned on the
output so a client can expect a response for every request sent.
davidkyle and others added 6 commits March 23, 2021 11:43
Evaluate a simple linear model for testing and another use-case
added to the repertoire
This commit changes the way the pytorch_inference process writes
results so they fit the format java expects.
Following #1841 the output is now an array of objects rather than NDJSON
Copy link
Contributor

@droberts195 droberts195 left a comment

Choose a reason for hiding this comment

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

LGTM if CI passes

@davidkyle davidkyle changed the title [ML] Merge the feature/pytorch_inference branch [ML] Merge the feature/pytorch-inference branch Jun 1, 2021
@davidkyle davidkyle merged commit e7a0342 into master Jun 1, 2021
@droberts195 droberts195 deleted the feature/pytorch-inference branch July 28, 2021 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants