Skip to content

Conversation

@larryliu0820
Copy link
Contributor

@larryliu0820 larryliu0820 commented Oct 2, 2025

This pull request enhances the deserialization and restoration logic for named data segments in programs, ensuring that named data is reconstructed into a dedicated store and verified in tests. It also includes minor code quality improvements.

Named Data Segment Restoration:

  • In _restore_segments (exir/_serialize/_program.py), added logic to reconstruct named data blobs into a NamedDataStoreOutput and attach both named_data_store and a convenient named_data_blobs mapping to the Program object. The original named_data list is cleared after restoration.
  • Updated tests (test_constant_delegate_and_named_data_segments and test_named_data_segments in exir/_serialize/test/test_program.py) to verify that named data is correctly restored, removed from the Program, and accessible via the new store and blob mapping. [1] [2]

@pytorch-bot
Copy link

pytorch-bot bot commented Oct 2, 2025

🔗 Helpful Links

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

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

❌ 1 New Failure, 1 Unrelated Failure

As of commit 5edbffd with merge base f24351a (image):

NEW FAILURE - The following job has failed:

BROKEN TRUNK - The following job failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

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

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 2, 2025
@github-actions
Copy link

github-actions bot commented Oct 2, 2025

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

@larryliu0820 larryliu0820 requested a review from lucylq October 2, 2025 07:04
)
# Keep a convenient mapping from key to raw bytes for callers that only
# need to read the blobs.
setattr( # noqa: B010
Copy link
Contributor

Choose a reason for hiding this comment

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

Think we don't need this if we have the named_data_store in line 683.

Not sure about attaching named_data_store to program type though?

if isinstance(value, str):
data[key] = T[value]
else:
data[key] = T(value)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this change?

{key: buffers[idx].buffer for key, idx in pte_named_data.items()},
)
self.assertTrue(hasattr(program2, "named_data_blobs"))
self.assertEqual(program2.named_data_blobs, restored_named_data)
Copy link
Contributor

Choose a reason for hiding this comment

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

Test the roundtrip --> can re-serialize the program with program2.named_data_blobs?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants