Skip to content

Conversation

dhruvbird
Copy link
Contributor

@dhruvbird dhruvbird commented Sep 9, 2022

Stack from ghstack (oldest at bottom):

Summary: Currently, the model tracer build is broken because of 2 reasons:

  1. A few source files are missing, resulting in missing link time symbols
  2. The TRACING_BASED flag isn't passed correctly from the command line (specified as an evnironment variable) as a CMake flag

Both these issues were fixed.

Test Plan: Ran this command: USE_CUDA=0 TRACING_BASED=1 python setup.py develop --cmake

and saw that the tracer binary was built at build/bin/model_tracer - also ran it to ensure that it can generate a YAML file.

Differential Revision: D39391270

Summary: Currently, the model tracer build is broken because of 2 reasons:
1. A few source files are missing, resulting in missing link time symbols
2. The `TRACING_BASED` flag isn't passed correctly from the command line (specified as an evnironment variable) as a CMake flag

Both these issues were fixed.

Test Plan: Ran this command: `USE_CUDA=0 TRACING_BASED=1 python setup.py develop --cmake`

and saw that the tracer binary was built at `build/bin/model_tracer` - also ran it to ensure that it can generate a YAML file.

[ghstack-poisoned]
@pytorch-bot
Copy link

pytorch-bot bot commented Sep 9, 2022

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/84755

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit 8b3a01c:
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

dhruvbird added a commit that referenced this pull request Sep 9, 2022
Summary: Currently, the model tracer build is broken because of 2 reasons:
1. A few source files are missing, resulting in missing link time symbols
2. The `TRACING_BASED` flag isn't passed correctly from the command line (specified as an evnironment variable) as a CMake flag

Both these issues were fixed.

Test Plan: Ran this command: `USE_CUDA=0 TRACING_BASED=1 python setup.py develop --cmake`

and saw that the tracer binary was built at `build/bin/model_tracer` - also ran it to ensure that it can generate a YAML file.

ghstack-source-id: c7481f8
Pull Request resolved: #84755
Comment on lines +1036 to +1038
message(STATUS "Tracing Based Flag: ${TRACING_BASED}")
message(STATUS "Build Lite Interpreter Flag: ${BUILD_LITE_INTERPRETER}")
message(STATUS "Intern Build Mobile Flag: ${INTERN_BUILD_MOBILE}")
Copy link
Contributor

Choose a reason for hiding this comment

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

Delete debugging code

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 saw a bunch of places in existing that print the relevant flags where they are used as STATUS, so I wanted to keep there here - do you think that's okay?

@@ -230,6 +230,7 @@ def generate(
"OPENSSL_ROOT_DIR",
"STATIC_DISPATCH_BACKEND",
"SELECTED_OP_LIST",
"TRACING_BASED",
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice finding!

@dhruvbird
Copy link
Contributor Author

@dhruvbird has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@dhruvbird
Copy link
Contributor Author

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

@pytorchbot successfully started a merge job. Check the current status here.
The merge job was triggered without a flag. This means that your change will be merged once all checks on your PR have passed (ETA: 0-4 Hours). If this is not the intended behavior, feel free to use some of the other merge options in the wiki.
Please reach out to the PyTorch DevX Team with feedback or questions!

@github-actions
Copy link
Contributor

github-actions bot commented Sep 9, 2022

Hey @dhruvbird.
You've committed this PR, but it does not have both a 'release notes: ...' and 'topics: ...' label. Please add one of each to the PR. The 'release notes: ...' label should represent the part of PyTorch that this PR changes (fx, autograd, distributed, etc) and the 'topics: ...' label should represent the kind of PR it is (not user facing, new feature, bug fix, perf improvement, etc). The list of valid labels can be found here for the 'release notes: ...' and here for the 'topics: ...'.
For changes that are 'topic: not user facing' there is no need for a release notes label.

@dhruvbird dhruvbird added release notes: mobile release notes category topic: bug fixes topic category topic: build labels Sep 9, 2022
dhruvbird added a commit that referenced this pull request Sep 10, 2022
…ake/Summary.cmake

Summary: In [PR 84755](#84755), @cccclai noticed and mentioned the presence of `message(STATUS...)` logging in caffe2/CMakeLists.txt and suggested moving it to the file cmake/Summary.cmake. This PR addresses that comment/suggestion.

Test Plan: Ran the build as `USE_NUMPY=0 USE_DISTRIBUTED=0 USE_CUDA=0 TRACING_BASED=1 python setup.py develop`

and saw the follwing being printed:

```
--   BUILD_MOBILE_AUTOGRAD : OFF
--   BUILD_LITE_INTERPRETER: OFF
--   INTERN_BUILD_MOBILE   :
--   TRACING_BASED         : 1
```

[ghstack-poisoned]
dhruvbird added a commit that referenced this pull request Sep 10, 2022
…ists.txt with message in cmake/Summary.cmake"

Summary: In [PR 84755](#84755), cccclai noticed and mentioned the presence of `message(STATUS...)` logging in caffe2/CMakeLists.txt and suggested moving it to the file cmake/Summary.cmake. This PR addresses that comment/suggestion.

Test Plan: Ran the build as `USE_NUMPY=0 USE_DISTRIBUTED=0 USE_CUDA=0 TRACING_BASED=1 python setup.py develop`

and saw the follwing being printed:

```
--   BUILD_MOBILE_AUTOGRAD : OFF
--   BUILD_LITE_INTERPRETER: OFF
--   INTERN_BUILD_MOBILE   :
--   TRACING_BASED         : 1
```

[ghstack-poisoned]
dhruvbird added a commit that referenced this pull request Sep 10, 2022
…ake/Summary.cmake

Summary: In [PR 84755](#84755), cccclai noticed and mentioned the presence of `message(STATUS...)` logging in caffe2/CMakeLists.txt and suggested moving it to the file cmake/Summary.cmake. This PR addresses that comment/suggestion.

Test Plan: Ran the build as `USE_NUMPY=0 USE_DISTRIBUTED=0 USE_CUDA=0 TRACING_BASED=1 python setup.py develop`

and saw the follwing being printed:

```
--   BUILD_MOBILE_AUTOGRAD : OFF
--   BUILD_LITE_INTERPRETER: OFF
--   INTERN_BUILD_MOBILE   :
--   TRACING_BASED         : 1
```

ghstack-source-id: 006743b
Pull Request resolved: #84814
dhruvbird added a commit that referenced this pull request Sep 10, 2022
…ssage in cmake/Summary.cmake"

Summary: In [PR 84755](#84755), cccclai noticed and mentioned the presence of `message(STATUS...)` logging in caffe2/CMakeLists.txt and suggested moving it to the file cmake/Summary.cmake. This PR addresses that comment/suggestion.

Test Plan: Ran the build as `USE_NUMPY=0 USE_DISTRIBUTED=0 USE_CUDA=0 TRACING_BASED=1 python setup.py develop`

and saw the follwing being printed:

```
--   BUILD_MOBILE_AUTOGRAD : OFF
--   BUILD_LITE_INTERPRETER: OFF
--   INTERN_BUILD_MOBILE   :
--   TRACING_BASED         : 1
```

[ghstack-poisoned]
dhruvbird added a commit that referenced this pull request Sep 11, 2022
…ists.txt with message in cmake/Summary.cmake"

Summary: In [PR 84755](#84755), cccclai noticed and mentioned the presence of `message(STATUS...)` logging in caffe2/CMakeLists.txt and suggested moving it to the file cmake/Summary.cmake. This PR addresses that comment/suggestion.

Test Plan: Ran the build as `USE_NUMPY=0 USE_DISTRIBUTED=0 USE_CUDA=0 TRACING_BASED=1 python setup.py develop`

and saw the follwing being printed:

```
--   BUILD_MOBILE_AUTOGRAD : OFF
--   BUILD_LITE_INTERPRETER: OFF
--   INTERN_BUILD_MOBILE   :
--   TRACING_BASED         : 1
```

[ghstack-poisoned]
dhruvbird added a commit that referenced this pull request Sep 11, 2022
…ssage in cmake/Summary.cmake"

Summary: In [PR 84755](#84755), cccclai noticed and mentioned the presence of `message(STATUS...)` logging in caffe2/CMakeLists.txt and suggested moving it to the file cmake/Summary.cmake. This PR addresses that comment/suggestion.

Test Plan: Ran the build as `USE_NUMPY=0 USE_DISTRIBUTED=0 USE_CUDA=0 TRACING_BASED=1 python setup.py develop`

and saw the follwing being printed:

```
--   BUILD_MOBILE_AUTOGRAD : OFF
--   BUILD_LITE_INTERPRETER: OFF
--   INTERN_BUILD_MOBILE   :
--   TRACING_BASED         : 1
```

[ghstack-poisoned]
facebook-github-bot pushed a commit that referenced this pull request Sep 11, 2022
Summary:
Currently, the model tracer build is broken because of 2 reasons:
1. A few source files are missing, resulting in missing link time symbols
2. The `TRACING_BASED` flag isn't passed correctly from the command line (specified as an evnironment variable) as a CMake flag

Both these issues were fixed.

Pull Request resolved: #84755
Approved by: https://github.com/cccclai

Test Plan:
contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/18a31cc0448f226f4c2dd9926d24aaef86409f1c

Test plan from GitHub:
Ran this command: `USE_CUDA=0 TRACING_BASED=1 python setup.py develop --cmake`

and saw that the tracer binary was built at `build/bin/model_tracer` - also ran it to ensure that it can generate a YAML file.

Original Phabricator Test Plan:
Ran this command: `USE_CUDA=0 TRACING_BASED=1 python setup.py develop --cmake`

and saw that the tracer binary was built at `build/bin/model_tracer` - also ran it to ensure that it can generate a YAML file.

Reviewed By: izaitsevfb, cccclai

Differential Revision: D39391270

Pulled By: dhruvbird

fbshipit-source-id: ff33f78b314a6fce96f4b9a99248815c251e0453
pytorchmergebot pushed a commit that referenced this pull request Sep 12, 2022
…ake/Summary.cmake (#84814)

Summary: In [PR 84755](#84755), @cccclai noticed and mentioned the presence of `message(STATUS...)` logging in caffe2/CMakeLists.txt and suggested moving it to the file cmake/Summary.cmake. This PR addresses that comment/suggestion.

Test Plan: Ran the build as `USE_NUMPY=0 USE_DISTRIBUTED=0 USE_CUDA=0 TRACING_BASED=1 python setup.py develop`

and saw the follwing being printed:

```
--   BUILD_MOBILE_AUTOGRAD : OFF
--   BUILD_LITE_INTERPRETER: OFF
--   INTERN_BUILD_MOBILE   :
--   TRACING_BASED         : 1
```
Pull Request resolved: #84814
Approved by: https://github.com/cccclai
Rick0317 pushed a commit to Rick0317/pytorch that referenced this pull request Oct 19, 2022
…ake/Summary.cmake

Summary: In [PR 84755](pytorch/pytorch#84755), cccclai noticed and mentioned the presence of `message(STATUS...)` logging in caffe2/CMakeLists.txt and suggested moving it to the file cmake/Summary.cmake. This PR addresses that comment/suggestion.

Test Plan: Ran the build as `USE_NUMPY=0 USE_DISTRIBUTED=0 USE_CUDA=0 TRACING_BASED=1 python setup.py develop`

and saw the follwing being printed:

```
--   BUILD_MOBILE_AUTOGRAD : OFF
--   BUILD_LITE_INTERPRETER: OFF
--   INTERN_BUILD_MOBILE   :
--   TRACING_BASED         : 1
```

ghstack-source-id: 2d49f53
Pull Request resolved: pytorch/pytorch#84814
Rick0317 pushed a commit to Rick0317/pytorch that referenced this pull request Oct 19, 2022
Pull Request resolved: pytorch/pytorch#84755


Currently, the model tracer build is broken because of 2 reasons:
1. A few source files are missing, resulting in missing link time symbols
2. The `TRACING_BASED` flag isn't passed correctly from the command line (specified as an evnironment variable) as a CMake flag

Both these issues were fixed.

Differential Revision: [D39391270](https://our.internmc.facebook.com/intern/diff/D39391270/)

**NOTE FOR REVIEWERS**: This PR has internal Facebook specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D39391270/)!
ghstack-source-id: 166946891
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.

4 participants