Get, save, and load module information for each operator #42133

taivu1998 · 2020-07-27T21:35:45Z

Stack from ghstack:

Get, save, and load module information for each operator #42133 Get, save, and load module information for each operator
Reconstruct scopes #41615 Reconstruct scopes

Differential Revision: D22803740

[ghstack-poisoned]

dr-ci · 2020-07-27T21:37:21Z

💊 CI failures summary and remediations

As of commit da42c7c (more details on the Dr. CI page):

4/5 failures possibly* introduced in this PR
- 1/4 non-CircleCI failure(s)
1/5 broken upstream at merge base c9dcc83 since Aug 12

🕵️ 3 new failures recognized by patterns

The following CI failures do not appear to be due to upstream breakages:

caffe2_onnx_main_py3_6_clang7_ubuntu16_04_test (1/3)

Step: "Test" (full log | diagnosis details | 🔁 rerun)

Aug 13 08:44:26 ERROR: No matching distribution found for ort-nightly==1.4.0.dev202007311

Aug 13 08:44:23 The directory '/var/lib/jenkins/.cache/pip' or its parent directory is not owned by the current user and caching wheels has been disabled. check the permissions and owner of that directory. If executing pip with sudo, you may want sudo's -H flag. 
Aug 13 08:44:23 Collecting pip 
Aug 13 08:44:24   Downloading https://files.pythonhosted.org/packages/5a/4a/39400ff9b36e719bdf8f31c99fe1fa7842a42fa77432e584f707a5080063/pip-20.2.2-py2.py3-none-any.whl (1.5MB) 
Aug 13 08:44:24 Installing collected packages: pip 
Aug 13 08:44:24   Found existing installation: pip 9.0.1 
Aug 13 08:44:24     Uninstalling pip-9.0.1: 
Aug 13 08:44:24       Successfully uninstalled pip-9.0.1 
Aug 13 08:44:25 Successfully installed pip-20.2.2 
Aug 13 08:44:25 + pip install -q --user -i https://test.pypi.org/simple/ ort-nightly==1.4.0.dev202007311 
Aug 13 08:44:26 ERROR: Could not find a version that satisfies the requirement ort-nightly==1.4.0.dev202007311 (from versions: 1.4.0.dev202007271) 
Aug 13 08:44:26 ERROR: No matching distribution found for ort-nightly==1.4.0.dev202007311

caffe2_onnx_ort1_py3_6_clang7_ubuntu16_04_test (2/3)

Step: "Test" (full log | diagnosis details | 🔁 rerun)

Aug 13 08:47:35 ERROR: No matching distribution found for ort-nightly==1.4.0.dev202007311

Aug 13 08:47:32 The directory '/var/lib/jenkins/.cache/pip' or its parent directory is not owned by the current user and caching wheels has been disabled. check the permissions and owner of that directory. If executing pip with sudo, you may want sudo's -H flag. 
Aug 13 08:47:33 Collecting pip 
Aug 13 08:47:33   Downloading https://files.pythonhosted.org/packages/5a/4a/39400ff9b36e719bdf8f31c99fe1fa7842a42fa77432e584f707a5080063/pip-20.2.2-py2.py3-none-any.whl (1.5MB) 
Aug 13 08:47:33 Installing collected packages: pip 
Aug 13 08:47:33   Found existing installation: pip 9.0.1 
Aug 13 08:47:33     Uninstalling pip-9.0.1: 
Aug 13 08:47:34       Successfully uninstalled pip-9.0.1 
Aug 13 08:47:35 Successfully installed pip-20.2.2 
Aug 13 08:47:35 + pip install -q --user -i https://test.pypi.org/simple/ ort-nightly==1.4.0.dev202007311 
Aug 13 08:47:35 ERROR: Could not find a version that satisfies the requirement ort-nightly==1.4.0.dev202007311 (from versions: 1.4.0.dev202007271) 
Aug 13 08:47:35 ERROR: No matching distribution found for ort-nightly==1.4.0.dev202007311

caffe2_onnx_ort2_py3_6_clang7_ubuntu16_04_test (3/3)

Step: "Test" (full log | diagnosis details | 🔁 rerun)

Aug 13 08:46:42 ERROR: No matching distribution found for ort-nightly==1.4.0.dev202007311

Aug 13 08:46:39 The directory '/var/lib/jenkins/.cache/pip' or its parent directory is not owned by the current user and caching wheels has been disabled. check the permissions and owner of that directory. If executing pip with sudo, you may want sudo's -H flag. 
Aug 13 08:46:39 Collecting pip 
Aug 13 08:46:39   Downloading https://files.pythonhosted.org/packages/5a/4a/39400ff9b36e719bdf8f31c99fe1fa7842a42fa77432e584f707a5080063/pip-20.2.2-py2.py3-none-any.whl (1.5MB) 
Aug 13 08:46:40 Installing collected packages: pip 
Aug 13 08:46:40   Found existing installation: pip 9.0.1 
Aug 13 08:46:40     Uninstalling pip-9.0.1: 
Aug 13 08:46:40       Successfully uninstalled pip-9.0.1 
Aug 13 08:46:41 Successfully installed pip-20.2.2 
Aug 13 08:46:41 + pip install -q --user -i https://test.pypi.org/simple/ ort-nightly==1.4.0.dev202007311 
Aug 13 08:46:42 ERROR: Could not find a version that satisfies the requirement ort-nightly==1.4.0.dev202007311 (from versions: 1.4.0.dev202007271) 
Aug 13 08:46:42 ERROR: No matching distribution found for ort-nightly==1.4.0.dev202007311

🚧 1 ongoing upstream failure:

These were probably caused by upstream breakages that are not fixed yet:

pytorch_linux_xenial_cuda10_2_cudnn7_py3_gcc7_test since Aug 12
- 🔁 rerun

ci.pytorch.org: 1 failed

Failed: pr/pytorch-linux-xenial-rocm3.5.1-py3.6

This comment was automatically generated by Dr. CI (expand for details).

Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions on the GitHub issue tracker or post in the (internal) Dr. CI Users group.

See how this bot performed.

This comment has been revised 119 times.

[ghstack-poisoned]

ghstack-source-id: 8b71878 Pull Request resolved: #42133

[ghstack-poisoned]

ghstack-source-id: f6becbe Pull Request resolved: #42133

[ghstack-poisoned]

ghstack-source-id: 746a334 Pull Request resolved: #42133

[ghstack-poisoned]

ghstack-source-id: 746a334 Pull Request resolved: #42133

torch/csrc/jit/passes/reconstruct_scopes.cpp

kimishpatel · 2020-07-29T15:09:32Z

torch/csrc/jit/passes/reconstruct_scopes.cpp

+  module_names[module._ivalue()] = getSubModuleName(module, prefix);
  for (NameModule s : module.named_children()) {
-    constructRelativeNamesForModules(s.value, prefix + "." + s.name);
+    std::string newPrefix = (!has_duplicated_function) ?


Instead of calling has_duplicated_function, maybe we can say class_types_are_not_unique. Also put a comment suggesting why is this necessary? Give an example, like when doing traced module vs scripted module with two instances of same type what can happen, and what we are doing for that.

kimishpatel · 2020-07-29T15:10:07Z

torch/csrc/jit/passes/reconstruct_scopes.cpp

  for (NameModule s : module.named_children()) {
-    constructRelativeNamesForModules(s.value, prefix + "." + s.name);
+    std::string newPrefix = (!has_duplicated_function) ?
+        module_names[module._ivalue()] + "." + s.name : module_names[module._ivalue()] + ".";


When has_duplicated_function=true, I suggest add something like .

Can you clarify your suggestion a bit? Also, since we decided not to save module info the two cases differently, I believe that we don't need to check for has_duplicated_function here. Is that correct?

Can you clarify your suggestion a bit? Also, since we decided not to save module info the two cases differently, I believe that we don't need to check for has_duplicated_function here. Is that correct?

Are you talking about not checking has_duplicated_function for this PR or after the other PR

When has_duplicated_function=true, I suggest add something like .

My first comment had trail off. I meant when has_duplicated_function=true, add "." or something like that.

kimishpatel · 2020-07-29T15:14:04Z

torch/csrc/jit/passes/reconstruct_scopes.cpp

+  if (has_duplicated_function) {
+    SourceRange r = frame.second;
+    if (r.source()) {
+      if (auto orig = r.source()->findSourceRangeThatGenerated(r)) {
+        r = *orig;
+      }
+    }
+    if (auto file_line_col = r.file_line_col()) {
+      std::string filename;
+      size_t line, col;
+      std::tie(filename, line, col) = *file_line_col;
+      scopeString += "(" + filename + ":" + c10::to_string(line) + ":" + c10::to_string(col) + ")";
+    }
+  }


Not clear to me why we should do this? If we decide that this might help, then it is probably best to split it out into a helper function with comments and fn name that suggests what it will do.
But let's first clarify what does this do.

That was for adding source range info in the case of a scripted module, as @ZolotukhinM suggested before. But I think I'm gonna remove it now as we decided not to do things differently for the two types of modules.

Make it a separate function in that case, atleast, till we resolve it.

kimishpatel · 2020-07-29T15:16:52Z

torch/csrc/jit/passes/reconstruct_scopes.cpp


-  visitBlock(graph->block());
+  if (has_duplicated_function) {
+    std::cout << "WARNING: There are some duplicated module instances." << std::endl;


This is not clear. Maybe say your module seems to have two instances of the same type. Current debugging does not support distinguishing the two instances of the same type yet. Thus debugging information maybe incomplete. Or something to the effect.

Yes, I'll fix that. One small question: Since it's a warning, is it ok to directly print it directly to std::cout like this?

Good question. I missed this. I think there is something like TORCH_WARN or torch_debug soemthing. I will have to check.

torch/csrc/jit/serialization/export_module.cpp

Differential Revision: [D22803740](https://our.internmc.facebook.com/intern/diff/D22803740) [ghstack-poisoned]

ghstack-source-id: 306aa75 Pull Request resolved: #42133

torch/csrc/jit/serialization/export_module.cpp

torch/csrc/jit/mobile/import.cpp

kimishpatel

This is really good Tai. Thanks for making this change.

Differential Revision: [D22803740](https://our.internmc.facebook.com/intern/diff/D22803740) [ghstack-poisoned]

iseeyuan · 2020-08-12T20:46:07Z

torch/csrc/jit/python/script_init.cpp

          py::arg("filename"),
-          py::arg("_extra_files") = ExtraFilesMap())
+          py::arg("_extra_files") = ExtraFilesMap(),
+          py::arg("_save_debug_info_in_bytecode") = false)


Is this covered by tests from Python side? The tests could be added in test/mobile/test_lite_script_module.py.

I added Python tests. Since we have no access to the internal C++ object, I basically check if the outputs still match when we enable debug info saving and if module info strings are contained in the debug info buffer. Let me know if that suffices.

Differential Revision: [D22803740](https://our.internmc.facebook.com/intern/diff/D22803740) [ghstack-poisoned]

torch/csrc/jit/serialization/export_module.cpp

test/mobile/test_lite_script_module.py

Differential Revision: [D22803740](https://our.internmc.facebook.com/intern/diff/D22803740) [ghstack-poisoned]

ghstack-source-id: 94f20b7 Pull Request resolved: #42133

torch/csrc/jit/mobile/import.cpp

Differential Revision: [D22803740](https://our.internmc.facebook.com/intern/diff/D22803740) [ghstack-poisoned]

ghstack-source-id: aa1095f Pull Request resolved: #42133

facebook-github-bot · 2020-08-14T10:13:18Z

@taivu1998 merged this pull request in ccd9f32.

Get, save, and load module information for each operator

773b4a5

[ghstack-poisoned]

taivu1998 requested a review from apaszke as a code owner July 27, 2020 21:35

taivu1998 mentioned this pull request Jul 27, 2020

Reconstruct scopes #41615

Closed

facebook-github-bot added the oncall: jit Add this issue/PR to JIT oncall triage queue label Jul 27, 2020

taivu1998 requested a review from kimishpatel July 27, 2020 21:37

Update on "Get, save, and load module information for each operator"

828e035

[ghstack-poisoned]

Update on "Get, save, and load module information for each operator"

699ca51

[ghstack-poisoned]

Update on "Get, save, and load module information for each operator"

ab9fb5a

[ghstack-poisoned]

taivu1998 pushed a commit that referenced this pull request Jul 27, 2020

Get, save, and load module information for each operator

f00a50d

ghstack-source-id: 8b71878 Pull Request resolved: #42133

Update on "Get, save, and load module information for each operator"

6d90a0e

[ghstack-poisoned]

Update on "Get, save, and load module information for each operator"

4fe1439

[ghstack-poisoned]

taivu1998 pushed a commit that referenced this pull request Jul 28, 2020

Get, save, and load module information for each operator

c2597dc

ghstack-source-id: f6becbe Pull Request resolved: #42133

Update on "Get, save, and load module information for each operator"

a4f75a9

[ghstack-poisoned]

taivu1998 pushed a commit that referenced this pull request Jul 28, 2020

Get, save, and load module information for each operator

91ea539

ghstack-source-id: 746a334 Pull Request resolved: #42133

taivu1998 requested a review from ZolotukhinM July 28, 2020 05:51

Update on "Get, save, and load module information for each operator"

4b59905

[ghstack-poisoned]

taivu1998 pushed a commit that referenced this pull request Jul 28, 2020

Get, save, and load module information for each operator

7223c6e

ghstack-source-id: 746a334 Pull Request resolved: #42133

taivu1998 requested a review from iseeyuan July 28, 2020 20:06

kimishpatel reviewed Jul 29, 2020

View reviewed changes

torch/csrc/jit/passes/reconstruct_scopes.cpp Outdated Show resolved Hide resolved

kimishpatel reviewed Jul 29, 2020

View reviewed changes

torch/csrc/jit/serialization/export_module.cpp Outdated Show resolved Hide resolved

kimishpatel reviewed Jul 29, 2020

View reviewed changes

torch/csrc/jit/serialization/export_module.cpp Show resolved Hide resolved

Update on "Get, save, and load module information for each operator"

1d98232

Differential Revision: [D22803740](https://our.internmc.facebook.com/intern/diff/D22803740) [ghstack-poisoned]

taivu1998 pushed a commit that referenced this pull request Aug 12, 2020

Get, save, and load module information for each operator

b383cf6

ghstack-source-id: 306aa75 Pull Request resolved: #42133

kimishpatel reviewed Aug 12, 2020

View reviewed changes

torch/csrc/jit/serialization/export_module.cpp Outdated Show resolved Hide resolved

kimishpatel reviewed Aug 12, 2020

View reviewed changes

torch/csrc/jit/serialization/export_module.cpp Outdated Show resolved Hide resolved

kimishpatel reviewed Aug 12, 2020

View reviewed changes

torch/csrc/jit/serialization/export_module.cpp Outdated Show resolved Hide resolved

kimishpatel reviewed Aug 12, 2020

View reviewed changes

torch/csrc/jit/mobile/import.cpp Outdated Show resolved Hide resolved

kimishpatel requested changes Aug 12, 2020

View reviewed changes

kimishpatel requested a review from iseeyuan August 12, 2020 16:04

Update on "Get, save, and load module information for each operator"

92c4eaa

Differential Revision: [D22803740](https://our.internmc.facebook.com/intern/diff/D22803740) [ghstack-poisoned]

iseeyuan reviewed Aug 12, 2020

View reviewed changes

Update on "Get, save, and load module information for each operator"

9f9f35b

Differential Revision: [D22803740](https://our.internmc.facebook.com/intern/diff/D22803740) [ghstack-poisoned]

Update on "Get, save, and load module information for each operator"

ce3aa45

Differential Revision: [D22803740](https://our.internmc.facebook.com/intern/diff/D22803740) [ghstack-poisoned]

taivu1998 requested a review from kimishpatel August 12, 2020 22:28

kimishpatel reviewed Aug 12, 2020

View reviewed changes

torch/csrc/jit/serialization/export_module.cpp Outdated Show resolved Hide resolved

kimishpatel reviewed Aug 12, 2020

View reviewed changes

test/mobile/test_lite_script_module.py Show resolved Hide resolved

Update on "Get, save, and load module information for each operator"

a290c41

Differential Revision: [D22803740](https://our.internmc.facebook.com/intern/diff/D22803740) [ghstack-poisoned]

taivu1998 pushed a commit that referenced this pull request Aug 13, 2020

Get, save, and load module information for each operator

6625b3c

ghstack-source-id: 94f20b7 Pull Request resolved: #42133

iseeyuan reviewed Aug 13, 2020

View reviewed changes

torch/csrc/jit/mobile/import.cpp Outdated Show resolved Hide resolved

Update on "Get, save, and load module information for each operator"

da42c7c

Differential Revision: [D22803740](https://our.internmc.facebook.com/intern/diff/D22803740) [ghstack-poisoned]

taivu1998 pushed a commit that referenced this pull request Aug 13, 2020

Get, save, and load module information for each operator

9f05b56

ghstack-source-id: aa1095f Pull Request resolved: #42133

facebook-github-bot closed this in ccd9f32 Aug 14, 2020

facebook-github-bot added the merged label Aug 14, 2020

facebook-github-bot deleted the gh/taivu1998/2/head branch August 17, 2020 14:16

mruberry added the Merged label Oct 28, 2020

Get, save, and load module information for each operator #42133

Get, save, and load module information for each operator #42133

Uh oh!

Conversation

taivu1998 commented Jul 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dr-ci bot commented Jul 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💊 CI failures summary and remediations

🕵️ 3 new failures recognized by patterns

caffe2_onnx_main_py3_6_clang7_ubuntu16_04_test (1/3)

caffe2_onnx_ort1_py3_6_clang7_ubuntu16_04_test (2/3)

caffe2_onnx_ort2_py3_6_clang7_ubuntu16_04_test (3/3)

🚧 1 ongoing upstream failure:

ci.pytorch.org: 1 failed

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

taivu1998 Jul 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kimishpatel left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

facebook-github-bot commented Aug 14, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

taivu1998 commented Jul 27, 2020 •

edited

Loading

dr-ci bot commented Jul 27, 2020 •

edited

Loading

taivu1998 Jul 29, 2020 •

edited

Loading