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.

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

Add TVM to testing dockerfile #1480

Closed
wants to merge 1 commit into from
Closed

Add TVM to testing dockerfile #1480

wants to merge 1 commit into from

Conversation

nhynes
Copy link
Contributor

@nhynes nhynes commented Feb 19, 2019

No description provided.

@nhynes nhynes requested review from kostko and pro-wh February 19, 2019 00:23
@codecov
Copy link

codecov bot commented Feb 19, 2019 •
edited
Loading

Codecov Report

Merging #1480 into master will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1480      +/-   ##
==========================================
+ Coverage   55.66%   55.67%   +0.01%     
==========================================
  Files         155      155              
  Lines       13990    13990              
==========================================
+ Hits         7787     7789       +2     
+ Misses       5454     5452       -2     
  Partials      749      749
Impacted Files Coverage Δ
go/roothash/tests/tester.go 94.33% <0%> (-0.95%) ⬇️
go/scheduler/alg/greedy_subgraphs_scheduler.go 89.47% <0%> (+5.26%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 96a6e3e...e36a53f. Read the comment docs.

1 similar comment
@codecov
Copy link

codecov bot commented Feb 19, 2019

Codecov Report

Merging #1480 into master will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1480      +/-   ##
==========================================
+ Coverage   55.66%   55.67%   +0.01%     
==========================================
  Files         155      155              
  Lines       13990    13990              
==========================================
+ Hits         7787     7789       +2     
+ Misses       5454     5452       -2     
  Partials      749      749
Impacted Files Coverage Δ
go/roothash/tests/tester.go 94.33% <0%> (-0.95%) ⬇️
go/scheduler/alg/greedy_subgraphs_scheduler.go 89.47% <0%> (+5.26%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 96a6e3e...e36a53f. Read the comment docs.

pro-wh
pro-wh reviewed Feb 19, 2019
View reviewed changes
Copy link
Contributor

@pro-wh pro-wh left a comment •
edited
Loading

Choose a reason for hiding this comment

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

any context or justification for this addition?

edit: implementation looks nothing too crazy

docker/testing/Dockerfile
# Install TVM
RUN git clone --recursive https://github.com/dmlc/tvm.git /tmp/tvm && \
cd /tmp/tvm && \
git checkout f6be4d6 && \
Copy link
Contributor

Choose a reason for hiding this comment

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

jroesch authored and tqchen committed 4 hours ago

🔥

Copy link
Contributor Author

Choose a reason for hiding this comment

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

only because I'm about to merge apache/tvm#2616 soon, which might break things

Copy link
Contributor Author

Choose a reason for hiding this comment

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

also, the 0.5 release is stalled apache/tvm#2614

@nhynes
Copy link
Contributor Author

nhynes commented Feb 19, 2019

any context or justification for this addition?

@pro-wh oasislabs/e2e-tests#20

kostko
kostko requested changes Feb 19, 2019
View reviewed changes
Copy link
Member

@kostko kostko left a comment

Choose a reason for hiding this comment

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

I am against adding this into the ekiden testing image. Instead this should be in a separate image, possibly derived from the ekiden testing image, in a repository where it is used.

@nhynes nhynes closed this Feb 19, 2019
@pro-wh
Copy link
Contributor

pro-wh commented Feb 19, 2019

Thanks for the additional info.

@nhynes nhynes deleted the tvm-testing branch February 19, 2019 20:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pro-wh pro-wh pro-wh left review comments

@kostko kostko kostko requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nhynes @pro-wh @kostko