Skip to content
This repository has been archived by the owner on Jan 20, 2022. It is now read-only.

[GSC] Add dockerfile and manifest file for tensorflow ResNet50 and BE… #2571

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

sahason
Copy link
Contributor

@sahason sahason commented Jul 22, 2021

…RT models gsc image

Signed-off-by: Sonali Saha [email protected]

Description of the changes

TensorFlow examples for ResNet50 and BERT models. The samples run inference using pre-trained models in GSC.

How to test this PR?

Please follow steps present at Tools/gsc/test/tensorflow/README.md


This change is Reviewable

Copy link
Contributor

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewed 4 of 4 files at r1.
Reviewable status: all files reviewed, 25 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ) (waiting on @sahason)

a discussion (no related file):
With these new PRs on adding examples to GSC, we need to re-consider the directory hierarchy for the GSC tool. This PR adds the TensorFlow example under Tools/gsc/test -- which was originally designed to have Jenkins-only, rather simple tests. (Similar to main Graphene's graphene/LibOS/shim/test.)

But this TensorFlow example doesn't seem to fit the "Jenkins-only, rather simple" test. So I suggest to create a new directory Tools/gsc/Examples and put TensorFlow under it. So in the end it will look like this:

- gsc
    - Examples
        - TensorFlow
            - ubuntu18.04-tensorflow.manifest
            - ubuntu18.04-tensorflow-bert.dockerfile
            - ubuntu18.04-tensorflow-resnet50.dockerfile
            - README.md

@mkow @aneessahib @veenasai2 What do you think about this? Also recall that GSC will soon be moved into its own repo.



Tools/gsc/test/ubuntu18.04-tensorflow-bert.dockerfile, line 6 at r1 (raw file):

RUN apt-get update \
    && apt-get install -y git wget \
    && apt-get install -y python3.6 python3-pip unzip \

Why not combine this and the previous lines into one?


Tools/gsc/test/ubuntu18.04-tensorflow-bert.dockerfile, line 6 at r1 (raw file):

RUN apt-get update \
    && apt-get install -y git wget \
    && apt-get install -y python3.6 python3-pip unzip \

Why specific version (python3.6), why not simply python3?


Tools/gsc/test/ubuntu18.04-tensorflow-bert.dockerfile, line 16 at r1 (raw file):

# Download data
RUN mkdir -p data \ 

Please remove trailing whitespace


Tools/gsc/test/ubuntu18.04-tensorflow-bert.dockerfile, line 25 at r1 (raw file):

    && wget https://storage.googleapis.com/intel-optimized-tensorflow/models/r2.5-icx-b631821f/asymmetric_per_channel_bert_int8.pb

ENTRYPOINT ["python3.6"]

Why not just "python3"? Why specific version?


Tools/gsc/test/ubuntu18.04-tensorflow-resnet50.dockerfile, line 6 at r1 (raw file):

RUN apt-get update \
    && apt-get install -y git wget \
    && apt-get install -y python3.6 python3-pip

Why specific version (python3.6), why not simply python3?


Tools/gsc/test/ubuntu18.04-tensorflow-resnet50.dockerfile, line 6 at r1 (raw file):

RUN apt-get update \
    && apt-get install -y git wget \
    && apt-get install -y python3.6 python3-pip

Why not combine this and the previous lines into one?


Tools/gsc/test/ubuntu18.04-tensorflow-resnet50.dockerfile, line 19 at r1 (raw file):

RUN git clone https://github.com/IntelAI/models.git /models/

ENTRYPOINT ["python3.6"]

Why not just "python3"? Why specific version?


Tools/gsc/test/tensorflow/README.md, line 1 at r1 (raw file):

# Inference on TensorFlow BERT and ResNet50 models:

This file is a lot of copy-paste from #2530.

I suggest to only keep the necessary parts in this README (Docker- and GSC-relevant), and add a link to that README here in the very beginning. Something like For additional information on how to install, run and optimize TensorFlow, please see <link to that README>.


Tools/gsc/test/tensorflow/README.md, line 6 at r1 (raw file):

inference. We tested this on Ubuntu 18.04 and uses the package version with Python 3.6.

## Bidirectional Encoder Representations from Transformers (BERT):

This item is already described in #2530, I would remove it from here and add a link to that README in this README (see my comment above).


Tools/gsc/test/tensorflow/README.md, line 13 at r1 (raw file):

https://github.com/google-research/bert.

## Residual Network (ResNet):

This item is already described in #2530, I would remove it from here and add a link to that README in this README (see my comment above).


Tools/gsc/test/tensorflow/README.md, line 18 at r1 (raw file):

https://github.com/IntelAI/models/tree/icx-launch-public/benchmarks/image_recognition/tensorflow/resnet50v1_5.

## Pre-System setting:

This item is already described in #2530, I would remove it from here and add a link to that README in this README (see my comment above).


Tools/gsc/test/tensorflow/README.md, line 34 at r1 (raw file):

Manually adopt config.yaml to the installed Intel SGX driver and desired Graphene repository/version

3. Generate the signing key: ``openssl genrsa -3 -out enclave-key.pem 3072``

These steps are not really needed to be described here. These steps are GSC-generic, so if a user reads this, he/she already knows how GSC works and how it is configured. Just removed this sub-section.


Tools/gsc/test/tensorflow/README.md, line 39 at r1 (raw file):

1. Build docker image:

cd test

This won't be needed because everything will be in the same dir.


Tools/gsc/test/tensorflow/README.md, line 41 at r1 (raw file):

cd test
docker build --rm -t ubuntu18.04-tensorflow-bert -f ubuntu18.04-tensorflow-bert.dockerfile \
../../../Examples

What is this directory? Looks like some remnant from a previous incarnation of this PR (where Examples contained both non-GSC and GSC files).


Tools/gsc/test/tensorflow/README.md, line 47 at r1 (raw file):

cd ..
./gsc build --insecure-args ubuntu18.04-tensorflow-bert test/ubuntu18.04-tensorflow.manifest

test/... -- This will be a different dir.


Tools/gsc/test/tensorflow/README.md, line 77 at r1 (raw file):

  1. To run int8 inference on native container:

This is too much copy-paste from the previous item. Please check how we changed it in #2530, and do the same here.


Tools/gsc/test/tensorflow/README.md, line 99 at r1 (raw file):

  1. Above commands are for a 36 core system. Please set the following options accordingly for

This item is already described in #2530, I would remove it from here and add a link to that README in this README (see my comment above).


Tools/gsc/test/tensorflow/README.md, line 113 at r1 (raw file):

1. Build docker image:

cd test

This won't be needed because everything will be in the same dir.


Tools/gsc/test/tensorflow/README.md, line 115 at r1 (raw file):

cd test
docker build --rm -t ubuntu18.04-tensorflow-resnet50 -f ubuntu18.04-tensorflow-resnet50.dockerfile \
../../../Examples

What is this directory? Looks like some remnant from a previous incarnation of this PR (where Examples contained both non-GSC and GSC files).


Tools/gsc/test/tensorflow/README.md, line 120 at r1 (raw file):

2. Graphenize the docker image using gsc build:
```cd ..
./gsc build --insecure-args ubuntu18.04-tensorflow-resnet50 test/ubuntu18.04-tensorflow.manifest

test/... -- This will be a different dir.


Tools/gsc/test/tensorflow/README.md, line 141 at r1 (raw file):

--steps=500

NOTE: When OOM happens user can set environment varibale TF_MKL_ALLOC_MAX_BYTES to upper

Please remove trailing spaces from everywhere here.


Tools/gsc/test/tensorflow/README.md, line 145 at r1 (raw file):

``--env TF_MKL_ALLOC_MAX_BYTES=17179869184`` to docker run command when OOM happens.

5. To run inference on native Container:

This is too much copy-paste from the previous item. Please check how we changed it in #2530, and do the same here.


Tools/gsc/test/tensorflow/README.md, line 159 at r1 (raw file):

  1. Above commands are for a 36 core system. Please set the following options accordingly for

This item is already described in #2530, I would remove it from here and add a link to that README in this README (see my comment above).


Tools/gsc/test/tensorflow/README.md, line 171 at r1 (raw file):

    KMP_AFFINITY binds OpenMP threads to physical processing units.

## Performance considerations:

This item is already described in #2530, I would remove it from here and add a link to that README in this README (see my comment above).

Copy link
Contributor

@aneessahib aneessahib left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 25 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ) (waiting on @mkow, @sahason, and @veenasai2)

a discussion (no related file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

With these new PRs on adding examples to GSC, we need to re-consider the directory hierarchy for the GSC tool. This PR adds the TensorFlow example under Tools/gsc/test -- which was originally designed to have Jenkins-only, rather simple tests. (Similar to main Graphene's graphene/LibOS/shim/test.)

But this TensorFlow example doesn't seem to fit the "Jenkins-only, rather simple" test. So I suggest to create a new directory Tools/gsc/Examples and put TensorFlow under it. So in the end it will look like this:

- gsc
    - Examples
        - TensorFlow
            - ubuntu18.04-tensorflow.manifest
            - ubuntu18.04-tensorflow-bert.dockerfile
            - ubuntu18.04-tensorflow-resnet50.dockerfile
            - README.md

@mkow @aneessahib @veenasai2 What do you think about this? Also recall that GSC will soon be moved into its own repo.

Yes makes sense.


Copy link
Contributor

@veenasai2 veenasai2 left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 25 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ) (waiting on @mkow and @sahason)

a discussion (no related file):

Previously, aneessahib wrote…

Yes makes sense.

Agree, that would be better organisation. Do we still need to test all the example gsc-images as part of https://github.com/oscarlab/graphene/blob/master/Tools/gsc/test/Makefile ?


Copy link
Contributor

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 25 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ) (waiting on @mkow and @sahason)

a discussion (no related file):

Previously, veenasai2 wrote…

Agree, that would be better organisation. Do we still need to test all the example gsc-images as part of https://github.com/oscarlab/graphene/blob/master/Tools/gsc/test/Makefile ?

As I mentioned at some point, I dislike the current state of that Makefile. I would prefer to have a much simpler Bash script / Python script, and maybe only keep a couple of smaller tests there (like Python and Bash examples, but not e.g. RA-TLS and Pytorch and Node.js). So I would move all the "big" tests in their own Examples/ directories.


Copy link
Contributor

@veenasai2 veenasai2 left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 25 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ) (waiting on @mkow and @sahason)

a discussion (no related file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

As I mentioned at some point, I dislike the current state of that Makefile. I would prefer to have a much simpler Bash script / Python script, and maybe only keep a couple of smaller tests there (like Python and Bash examples, but not e.g. RA-TLS and Pytorch and Node.js). So I would move all the "big" tests in their own Examples/ directories.

Sounds good, thanks.


Copy link
Contributor Author

@sahason sahason left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 5 files reviewed, 25 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ), "fixup! " found in commit messages' one-liners (waiting on @dimakuv and @mkow)

a discussion (no related file):

Previously, veenasai2 wrote…

Agree, that would be better organisation. Do we still need to test all the example gsc-images as part of https://github.com/oscarlab/graphene/blob/master/Tools/gsc/test/Makefile ?

Done.



Tools/gsc/test/tensorflow/README.md, line 1 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

This file is a lot of copy-paste from #2530.

I suggest to only keep the necessary parts in this README (Docker- and GSC-relevant), and add a link to that README here in the very beginning. Something like For additional information on how to install, run and optimize TensorFlow, please see <link to that README>.

Done. For now I have given this link https://github.com/Satya1493/graphene/blob/tensorflow/Examples/tensorflow/README.md. I will change it to master branch once this PR is merged.


Tools/gsc/test/tensorflow/README.md, line 6 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

This item is already described in #2530, I would remove it from here and add a link to that README in this README (see my comment above).

Done.


Tools/gsc/test/tensorflow/README.md, line 13 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

This item is already described in #2530, I would remove it from here and add a link to that README in this README (see my comment above).

Done.


Tools/gsc/test/tensorflow/README.md, line 18 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

This item is already described in #2530, I would remove it from here and add a link to that README in this README (see my comment above).

Done.


Tools/gsc/test/tensorflow/README.md, line 34 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

These steps are not really needed to be described here. These steps are GSC-generic, so if a user reads this, he/she already knows how GSC works and how it is configured. Just removed this sub-section.

Done.


Tools/gsc/test/tensorflow/README.md, line 39 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

This won't be needed because everything will be in the same dir.

Done.


Tools/gsc/test/tensorflow/README.md, line 41 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

What is this directory? Looks like some remnant from a previous incarnation of this PR (where Examples contained both non-GSC and GSC files).

Done.


Tools/gsc/test/tensorflow/README.md, line 47 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

test/... -- This will be a different dir.

Done.


Tools/gsc/test/tensorflow/README.md, line 77 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

This is too much copy-paste from the previous item. Please check how we changed it in #2530, and do the same here.

Done.


Tools/gsc/test/tensorflow/README.md, line 99 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

This item is already described in #2530, I would remove it from here and add a link to that README in this README (see my comment above).

I have removed these lines from here. But I have given README link for #2530 here as well along with providing the link at top for better user guidance.


Tools/gsc/test/tensorflow/README.md, line 113 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

This won't be needed because everything will be in the same dir.

Done.


Tools/gsc/test/tensorflow/README.md, line 115 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

What is this directory? Looks like some remnant from a previous incarnation of this PR (where Examples contained both non-GSC and GSC files).

Done.


Tools/gsc/test/tensorflow/README.md, line 120 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

test/... -- This will be a different dir.

Done.


Tools/gsc/test/tensorflow/README.md, line 141 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Please remove trailing spaces from everywhere here.

Done.


Tools/gsc/test/tensorflow/README.md, line 145 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

This is too much copy-paste from the previous item. Please check how we changed it in #2530, and do the same here.

Done.


Tools/gsc/test/tensorflow/README.md, line 159 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

This item is already described in #2530, I would remove it from here and add a link to that README in this README (see my comment above).

I have removed these lines from here. But I have given README link for #2530 here as well along with providing the link at top for better user guidance.


Tools/gsc/test/tensorflow/README.md, line 171 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

This item is already described in #2530, I would remove it from here and add a link to that README in this README (see my comment above).

Done.


Tools/gsc/test/ubuntu18.04-tensorflow-bert.dockerfile, line 6 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Why not combine this and the previous lines into one?

Done.


Tools/gsc/test/ubuntu18.04-tensorflow-bert.dockerfile, line 6 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Why specific version (python3.6), why not simply python3?

Done.


Tools/gsc/test/ubuntu18.04-tensorflow-bert.dockerfile, line 16 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Please remove trailing whitespace

Done.


Tools/gsc/test/ubuntu18.04-tensorflow-bert.dockerfile, line 25 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Why not just "python3"? Why specific version?

Done.


Tools/gsc/test/ubuntu18.04-tensorflow-resnet50.dockerfile, line 6 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Why specific version (python3.6), why not simply python3?

Done.


Tools/gsc/test/ubuntu18.04-tensorflow-resnet50.dockerfile, line 6 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Why not combine this and the previous lines into one?

Done.


Tools/gsc/test/ubuntu18.04-tensorflow-resnet50.dockerfile, line 19 at r1 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Why not just "python3"? Why specific version?

Done.

@sahason
Copy link
Contributor Author

sahason commented Aug 18, 2021

@mkow I have addressed review comments. Please provide your inputs. If everything looks fine request to approve this PR.

Copy link
Member

@mkow mkow left a comment

Choose a reason for hiding this comment

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

Reviewed 5 of 5 files at r2.
Reviewable status: all files reviewed, 51 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ), "fixup! " found in commit messages' one-liners (waiting on @dimakuv and @sahason)

a discussion (no related file):
[GSC] Add dockerfile and manifest file for tensorflow ResNet50 and BERT models gsc image -> [GSC] Add TensorFlow example with ResNet50 and BERT models



Tools/gsc/Examples/tensorflow/README.md, line 1 at r2 (raw file):

# Inference on TensorFlow BERT and ResNet50 models:

You usually don't put colon at the end of the section headers and also we don't do that in other places.


Tools/gsc/Examples/tensorflow/README.md, line 1 at r2 (raw file):

# Inference on TensorFlow BERT and ResNet50 models:

Please put an empty line after each heading


Tools/gsc/Examples/tensorflow/README.md, line 3 at r2 (raw file):

# Inference on TensorFlow BERT and ResNet50 models:
For additional information on how to install, run and optimize TensorFlow, please see
https://github.com/Satya1493/graphene/blob/tensorflow/Examples/tensorflow/README.md.

Graphene shouldn't link to your private repo


Tools/gsc/Examples/tensorflow/README.md, line 5 at r2 (raw file):

https://github.com/Satya1493/graphene/blob/tensorflow/Examples/tensorflow/README.md.

## Build graphenize Docker image and run BERT inference:

Build -> Building
run -> running


Tools/gsc/Examples/tensorflow/README.md, line 11 at r2 (raw file):

gsc build

This should be wrapped in backticks


Tools/gsc/Examples/tensorflow/README.md, line 17 at r2 (raw file):

  1. Sign the graphenized Docker image using gsc sign-image:

ditto


Tools/gsc/Examples/tensorflow/README.md, line 28 at r2 (raw file):

gsc-ubuntu18.04-tensorflow-bert \
models/models/language_modeling/tensorflow/bert_large/inference/run_squad.py \
--init_checkpoint=data/bert_large_checkpoints/model.ckpt-3649 \

These shell listing are very hard to read, please reformat them with nicer wrapping and use Bash syntax highlighting.


Tools/gsc/Examples/tensorflow/README.md, line 44 at r2 (raw file):

  1. To run fp32 inference on native container (outside Graphene), remove

on native container -> in a native container


Tools/gsc/Examples/tensorflow/README.md, line 49 at r2 (raw file):

6. Above commands are for a 36 core system. Please check
https://github.com/Satya1493/graphene/blob/tensorflow/Examples/tensorflow/README.md for setting

ditto


Tools/gsc/Examples/tensorflow/README.md, line 52 at r2 (raw file):

different options for optimal performance.

## Build graphenize Docker image and run ResNet50 inference:

Build -> Building
run -> running


Tools/gsc/Examples/tensorflow/README.md, line 58 at r2 (raw file):

  1. Graphenize the docker image using gsc build:

ditto


Tools/gsc/Examples/tensorflow/README.md, line 64 at r2 (raw file):

  1. Sign the graphenized Docker image using gsc sign-image:

ditto


Tools/gsc/Examples/tensorflow/README.md, line 82 at r2 (raw file):

--steps=500

NOTE: When OOM happens user can set environment varibale TF_MKL_ALLOC_MAX_BYTES to upper

varibale -> variable


Tools/gsc/Examples/tensorflow/README.md, line 82 at r2 (raw file):

--steps=500

NOTE: When OOM happens user can set environment varibale TF_MKL_ALLOC_MAX_BYTES to upper

When OOM happens user can set -> If an OOM happens you can try setting


Tools/gsc/Examples/tensorflow/README.md, line 83 at r2 (raw file):

to upper bound on memory allocation

What's does this mean? Should this be set to maximum memory ever used by the app? Or available in the system? The example below doesn't make that any clearer to me...


Tools/gsc/Examples/tensorflow/README.md, line 84 at r2 (raw file):

docker run

This should be in backticks


Tools/gsc/Examples/tensorflow/README.md, line 86 at r2 (raw file):

``--env TF_MKL_ALLOC_MAX_BYTES=17179869184`` to docker run command when OOM happens.

5. To run int8 inference on native container (outside Graphene), remove

on native container -> in a native container


Tools/gsc/Examples/tensorflow/README.md, line 91 at r2 (raw file):

6. Above commands are for a 36 core system. Please check
https://github.com/Satya1493/graphene/blob/tensorflow/Examples/tensorflow/README.md for setting

ditto


Tools/gsc/Examples/tensorflow/ubuntu18.04-tensorflow.manifest, line 5 at r2 (raw file):

loader.pal_internal_mem_size = "64M"
loader.insecure__use_host_env = 1
sgx.allowed_files.tmp = "file:/tmp"

You should mount tmpfs instead, see https://graphene.readthedocs.io/en/latest/manifest-syntax.html#fs-mount-points.


Tools/gsc/Examples/tensorflow/ubuntu18.04-tensorflow-bert.dockerfile, line 1 at r2 (raw file):

From ubuntu:18.04

In all other places you used uppercase letters for Docker commands, please unify.


Tools/gsc/Examples/tensorflow/ubuntu18.04-tensorflow-bert.dockerfile, line 8 at r2 (raw file):

    && pip3 install --upgrade pip

# Install tensorflow

tensorflow -> TensorFlow


Tools/gsc/Examples/tensorflow/ubuntu18.04-tensorflow-bert.dockerfile, line 9 at r2 (raw file):

# Install tensorflow
RUN pip3 install intel-tensorflow-avx512==2.4.0

Why exactly this specific version?


Tools/gsc/Examples/tensorflow/ubuntu18.04-tensorflow-resnet50.dockerfile, line 1 at r2 (raw file):

From ubuntu:18.04

ditto


Tools/gsc/Examples/tensorflow/ubuntu18.04-tensorflow-resnet50.dockerfile, line 8 at r2 (raw file):

    && pip3 install --upgrade pip

# Install tensorflow

ditto


Tools/gsc/Examples/tensorflow/ubuntu18.04-tensorflow-resnet50.dockerfile, line 9 at r2 (raw file):

# Install tensorflow
RUN pip3 install intel-tensorflow-avx512==2.4.0

ditto

Copy link
Contributor

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewed 4 of 5 files at r2.
Reviewable status: all files reviewed, 30 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ), "fixup! " found in commit messages' one-liners (waiting on @mkow and @sahason)


Tools/gsc/Examples/tensorflow/README.md, line 3 at r2 (raw file):

Previously, mkow (Michał Kowalczyk) wrote…

Graphene shouldn't link to your private repo

This will be changed -- there is nowhere else to link currently, because TensorFlow example is itself a pending PR.


Tools/gsc/Examples/tensorflow/README.md, line 22 at r2 (raw file):

  1. To run fp32 inference on GSC:

on GSC -> in a graphenized container


Tools/gsc/Examples/tensorflow/README.md, line 50 at r2 (raw file):

6. Above commands are for a 36 core system. Please check
https://github.com/Satya1493/graphene/blob/tensorflow/Examples/tensorflow/README.md for setting
different options for optimal performance.

Step 6 is not really a "build step", it is just a generic note. Moreover, this step 6 is duplicated verbatim for BERT and ResnNet50. I suggest to remove this step and instead add a note at the very end of this file:

## Notes

The above `docker run` commands are for a 36-core system. Please check
https://github.com/Satya1493/graphene/blob/tensorflow/Examples/tensorflow/README.md
for an overview of options to achieve optimal performance on different systems.

Tools/gsc/Examples/tensorflow/README.md, line 69 at r2 (raw file):

  1. To run int8 inference on GSC:

on GSC -> in a graphenized container


Tools/gsc/Examples/tensorflow/README.md, line 90 at r2 (raw file):

``ubuntu18.04-tensorflow-resnet50`` in the above command.

6. Above commands are for a 36 core system. Please check

Remove this step, see my other comment.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants