Skip to content

reference tensor proto of tensorflow #1729

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

Merged
merged 9 commits into from
Feb 14, 2020
Merged

reference tensor proto of tensorflow #1729

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
33cef00
init
QiJune Feb 13, 2020
7b2462c
update
QiJune Feb 13, 2020
a9f8154
refine cmd
QiJune Feb 13, 2020
25f5e1a
polish again
QiJune Feb 13, 2020
dd4044c
test ci
QiJune Feb 13, 2020
eb4b5bb
test ci
QiJune Feb 13, 2020
da52590
trigger ci
QiJune Feb 13, 2020
f7c02a0
fix ci
QiJune Feb 13, 2020
6b605c2
follow comments
QiJune Feb 14, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 10 additions & 9 deletions elasticdl/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,21 +1,22 @@
PYTHON = python

EDL_PROTO_FILE = elasticdl/proto/elasticdl.proto

PROTO_PATH=elasticdl/proto
Copy link
Collaborator

Choose a reason for hiding this comment

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

I prefer we don't define this variable, as its naming doesn't add much information. Instead, we could literally use elasticdl/proto in the following rules.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

GO_FILE_DIR = elasticdl/pkg/proto
GO_PB_FILE = $(GO_FILE_DIR)/elasticdl.pb.go

CXX = g++
CAPI_DIR = elasticdl/pkg/kernel/capi

all: python_pb $(GO_PB_FILE) kernel
all: python_pb go_pb kernel

python_pb:
Copy link
Collaborator

@wangkuiyi wangkuiyi Feb 14, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

This target requires at least ${PROTO_PATH}/elasticdl.proto as its dependent.

python_pb: elasticdl/proto/elasticdl.proto
    ...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

$(PYTHON) -m grpc_tools.protoc -I . $(EDL_PROTO_FILE) --python_out=. --grpc_python_out=.
$(PYTHON) -m grpc_tools.protoc -I${TF_PATH} -I. \
${PROTO_PATH}/elasticdl.proto --python_out=. --grpc_python_out=.

$(GO_PB_FILE): $(EDL_PROTO_FILE)
go_pb:
Copy link
Collaborator

@wangkuiyi wangkuiyi Feb 14, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

This target requires at least one dependent ${PROTO_PATH}/elasticdl.proto

go_pb: elasticdl/proto/elasticdl.proto
    ...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

mkdir -p $(GO_FILE_DIR)
protoc -I . $< --go_out=plugins=grpc:.
protoc -I${TF_PATH} ${TF_PATH}/tensorflow/core/framework/types.proto --go_out=${GOPATH}/src
protoc -I${TF_PATH} ${TF_PATH}/tensorflow/core/framework/resource_handle.proto --go_out=${GOPATH}/src
protoc -I${TF_PATH} ${TF_PATH}/tensorflow/core/framework/tensor_shape.proto --go_out=${GOPATH}/src
protoc -I${TF_PATH} ${TF_PATH}/tensorflow/core/framework/tensor.proto --go_out=${GOPATH}/src
protoc -I${TF_PATH} -I${PROTO_PATH} ${PROTO_PATH}/elasticdl.proto --go_out=plugins=grpc:.

kernel:
${CXX} -O3 -std=c++11 ${CAPI_DIR}/kernel_api.cc -c -o ${CAPI_DIR}/libkernel_api.o
Expand Down
8 changes: 4 additions & 4 deletions elasticdl/docker/Dockerfile.dev
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,11 @@ RUN /install-protobuf.bash && rm /install-protobuf.bash

# Install elasticdl.org/elasticdl Go package
ENV ELASTICDLPATH $GOPATH/src/elasticdl.org/elasticdl
ENV TF_PATH /tmp/tensorflow
RUN cd /tmp && git clone --depth=1 https://github.com/tensorflow/tensorflow.git
Copy link
Collaborator

Choose a reason for hiding this comment

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

Adding this line would significantly slow down the Docker build speed. @QiJune please follow up #1731 with @typhoonzero to use Docker build cache as much as possible.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I would prefer to fix #1731 before merging this PR; otherwise, the merge of this PR would significantly slow down the CI until we fix #1731.

Copy link
Collaborator Author

@QiJune QiJune Feb 14, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@wangkuiyi I compare two CI time of two PRs, PR 1726 and current PR

  • PR 1726: 6min 15s + 17min 34s = 23min 49s
  • current PR: 6min 6s + 16min 49s = 22min 55s

22min 55s is smaller than 23min 49s. It seems that downloading tensorflow will not influence the CI speed.

I usegit clone --depth=1, so the downloading speed will be fast.

COPY . /tmp/elasticdl
RUN cd /tmp/elasticdl && make -f elasticdl/Makefile && \
mkdir -p $ELASTICDLPATH && \
RUN cd /tmp/elasticdl && mkdir -p $ELASTICDLPATH && \
make -f elasticdl/Makefile && \
cp -r /tmp/elasticdl/elasticdl/pkg $ELASTICDLPATH && \
cd $GOPATH/src/elasticdl.org/elasticdl && \
export GO111MODULE=on && \
go mod init elasticdl.org/elasticdl && \
go install ./... && cd / && rm -rf /tmp/elasticdl
2 changes: 1 addition & 1 deletion elasticdl/docker/scripts/install-go.bash
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ GO_MIRROR_URL=$1

curl --silent ${GO_MIRROR_URL}/go1.13.4.linux-amd64.tar.gz | tar -C /usr/local -xzf -

go env -w GO111MODULE=on
go env -w GOPROXY=https://goproxy.io,direct

go get github.com/golang/protobuf/protoc-gen-go
Expand All @@ -16,5 +15,6 @@ go get golang.org/x/tools/cmd/cover
go get github.com/mattn/goveralls
go get github.com/rakyll/gotest
go get github.com/wangkuiyi/goyaccfmt
go get github.com/stretchr/testify/assert

cp $GOPATH/bin/* /usr/local/bin/
1 change: 1 addition & 0 deletions elasticdl/proto/elasticdl.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
syntax = "proto3";

import "google/protobuf/empty.proto";
import "tensorflow/core/framework/tensor.proto";

package proto;
option go_package = "elasticdl/pkg/proto";
Expand Down