-
Notifications
You must be signed in to change notification settings - Fork 685
Add program-data separation to pybindings #13886
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,7 @@ | |
ModuleAddWithAttributes, | ||
ModuleChannelsLast, | ||
ModuleChannelsLastInDefaultOut, | ||
ModuleLinear, | ||
ModuleMulti, | ||
) | ||
from torch.export import export | ||
|
@@ -623,3 +624,35 @@ def test_method_method_meta(self) -> None: | |
self.assertEqual(output_tensor.is_memory_planned(), True) | ||
self.assertEqual(output_tensor.nbytes(), 16) | ||
self.assertEqual(str(output_tensor), tensor_info) | ||
|
||
def test_program_data_separation(self) -> None: | ||
eager_module = ModuleLinear() | ||
inputs = eager_module.get_inputs() | ||
exported_program = export(eager_module, inputs, strict=True) | ||
exec_program = to_edge(exported_program).to_executorch( | ||
config=ExecutorchBackendConfig( | ||
# Move all tensor data to '_default_external_constant' file. | ||
external_constants=True, | ||
) | ||
) | ||
|
||
import os | ||
import tempfile | ||
|
||
with tempfile.TemporaryDirectory() as tmpdir: | ||
pte_file = os.path.join(tmpdir, "linear.pte") | ||
with open(pte_file, "wb") as f: | ||
f.write(exec_program.buffer) | ||
|
||
ptd_file = os.path.join(tmpdir, "linear.ptd") | ||
with open(ptd_file, "wb") as ptd: | ||
tensor_data = bytes( | ||
exec_program._tensor_data.pop("_default_external_constant") | ||
) | ||
ptd.write(tensor_data) | ||
|
||
executorch_program = self.runtime._load_for_executorch(pte_file, ptd_file) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's fine to update the legacy API (methods that start with _) but we have modern pybindings APIs that we want users to use. Don't we need to update runtime.load_program(pte, ptd) too? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks Mergen, sorry for the late response. I think runtime.load_program is only intended to take a PTE file --> it understands and deserializes the PTE file format (not PTD). The C++ Program object doesn't hold a PTD file. We can potentially support this in load_method, which takes a NamedDataMap (from PTD) as an optional argument. We'd need to add pybindings on extension/flat_tensor to load the PTD file for this. Having it in Module may be OK at the moment, let me know what you think (I can add pybindings for extension/flat_tensor in a separate PR). |
||
|
||
expected = eager_module(inputs[0]) | ||
executorch_output = executorch_program.forward(inputs)[0] | ||
self.assertTrue(torch.allclose(expected, executorch_output)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably need to update the pyi
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, updated!