[pkg] Improve mocked module detection by combining mocked object errors with the rest of the errors in PackageExporter (#74315)

Summary:
Pull Request resolved: https://github.com/pytorch/pytorch/pull/74315

Now instead of spitting out a NotImplemented Error when the Package Exporter finds an object for a mocked module, it combines these errors with the rest of the Packaging Errors of PackageExporter to get something like

```
torch.package.package_exporter.PackagingError:
* Module was mocked out, but is still being used in the package.Please intern or extern the mocked modules if objects are supposed to be inthe package.
    package_a
      Context: Object(s) '['PackageASubpackageObject']' from module package_a was mocked out during packaging but is being used in resource - obj.pkl in package obj.
    package_a.subpackage
      Context: Object(s) '['PackageASubpackageObject']' from module package_a.subpackage was mocked out during packaging but is being used in resource - obj.pkl in package obj.
```

This makes it significantly easier to fix mocked object errors as they all should appear at once.

Test Plan: Imported from OSS

Reviewed By: d4l3k

Differential Revision: D34951973

Pulled By: PaliC

fbshipit-source-id: 01ee4ba3767967ef9a9bcd69ad86362ebc100b2d
(cherry picked from commit 900edd270ee8f5802fc6e56df08fff6b073ac6f2)
This commit is contained in:
Sahan Paliskara
2022-03-17 11:19:42 -07:00
committed by PyTorch MergeBot
parent 119df90ead
commit 96ed3320f8
2 changed files with 35 additions and 12 deletions

View File

@ -84,6 +84,11 @@ class PackagingErrorReason(Enum):
"Module did not match against any action pattern. Extern, mock, or intern it."
)
DENIED = "Module was denied by a pattern."
MOCKED_BUT_STILL_USED = (
"Module was mocked out, but is still being used in the package. "
"Please intern or extern the mocked modules if objects are supposed to be in "
"the package."
)
@dataclass
@ -586,7 +591,7 @@ class PackageExporter:
pickler.persistent_id = self._persistent_id
pickler.dump(obj)
data_value = data_buf.getvalue()
mocked_modules = defaultdict(list)
name_in_dependency_graph = f"<{package}.{resource}>"
self.dependency_graph.add_node(
name_in_dependency_graph,
@ -596,6 +601,15 @@ class PackageExporter:
)
def _check_mocked_error(module: Optional[str], field: Optional[str]):
"""
checks if an object (field) comes from a mocked module and then adds
the pair to mocked_modules which contains mocked modules paired with their
list of mocked objects present in the pickle.
We also hold the invariant that the first user defined rule that applies
to the module is the one we use.
"""
assert isinstance(module, str)
assert isinstance(field, str)
if self._can_implicitly_extern(module):
@ -603,15 +617,8 @@ class PackageExporter:
for pattern, pattern_info in self.patterns.items():
if pattern.matches(module):
if pattern_info.action == _ModuleProviderAction.MOCK:
raise NotImplementedError(
f"Object '{field}' from module {module} was mocked out during packaging "
f"but is being used in resource - {resource} in package {package}. "
"If this error is happening during 'save_pickle', please ensure that your "
"pickled object doesn't contain any mocked objects. Try interning or externing"
f"{module} if {field} is supposed to be in the package."
)
else:
return
mocked_modules[module].append(field)
return
if dependencies:
all_dependencies = []
@ -655,7 +662,23 @@ class PackageExporter:
_check_mocked_error(module, field)
for module_name in all_dependencies:
self.dependency_graph.add_edge(name_in_dependency_graph, module_name)
self.add_dependency(module_name)
""" If an object happens to come from a mocked module, then we collect these errors and spit them
out with the other errors found by package exporter.
"""
if module in mocked_modules:
assert isinstance(module, str)
fields = mocked_modules[module]
self.dependency_graph.add_node(
module_name,
action=_ModuleProviderAction.MOCK,
error=PackagingErrorReason.MOCKED_BUT_STILL_USED,
error_context=f"Object(s) '{fields}' from module `{module_name}` was mocked out during packaging "
f"but is being used in resource - `{resource}` in package `{package}`. ",
provided=True,
)
else:
self.add_dependency(module_name)
self._write(filename, data_value)