Change NamedTupleVariable implementation to subclass UserDefinedTupleVariable by morrison-turnansky · Pull Request #167468 · pytorch/pytorch

morrison-turnansky · 2025-11-10T16:49:20Z

Continuation of work from previous PR, see link for context #161645 (comment)

I think this PR is a step in that direction. There is probably some room for simplification.

At a high level, the new class NamedTupleVariable handles methods that branch on structseq or the more dynamic subclasses of namedtuple, and falls back to UserDefinedTupleVariable otherwise.

Please let me know what you think. @StrongerXi

cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @kadeng @chauhang @amjames @Lucaskabela @jataylo @chenyang78

pytorch-bot · 2025-11-10T16:49:24Z

🔗 Helpful Links

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

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit the bot commands wiki

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

✅ You can merge normally! (1 Unrelated Failure)

As of commit 992fe52 with merge base d2237eb ():

FLAKY - The following job failed but was likely due to flakiness present on trunk:

trunk / linux-jammy-rocm-py3.10 / test (distributed, 2, 3, linux.rocm.gpu.gfx942.4) (gh) (disabled by #129390)
test/distributed/_tools/test_fsdp2_mem_tracker.py::TestTrackerFullyShard1DTrainingCore::test_tracker_multi_group_eager

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

morrison-turnansky · 2025-11-10T16:51:17Z

I went through the repo and grabbed namedtuple tests. The following pass locally with python 3.13 (required for collections tests)
test/dynamo/test_misc.py::MiscTests::test_namedtuple1
test/dynamo/test_misc.py::MiscTests::test_namedtuple2
test/dynamo/test_misc.py::MiscTests::test_namedtuple3
test/dynamo/test_misc.py::MiscTests::test_namedtuple_class
test/dynamo/test_misc.py::MiscTests::test_namedtuple_with_custom_getitem
test/dynamo/test_misc.py::MiscTests::test_namedtuple_source_dynamic_attributes
test/dynamo/test_misc.py::MiscTests::test_namedtuple_sourceless_dynamic_attributes
test/dynamo/test_misc.py::MiscTests::test_sourceless_namedtuple
test/dynamo/test_misc.py::MiscTests::test_structseq1
test/dynamo/test_misc.py::MiscTests::test_structseq2
test/dynamo/test_functions.py::FunctionTests::test_namedtuple
test/dynamo/test_functions.py::FunctionTests::test_namedtuple_defaults
test/dynamo/test_functions.py::FunctionTests::test_namedtuple_replace
test/dynamo/test_functions.py::FunctionTests::test_namedtuple_fields
test/dynamo/test_functions.py::FunctionTests::test_namedtuple_user_methods
test/dynamo/test_functions.py::FunctionTests::test_generic_namedtuple_user_methods
test/dynamo/test_functions.py::FunctionTests::test_namedtuple_hasattr
test/dynamo/test_functions.py::FunctionTests::test_generic_namedtuple_hasattr
test/dynamo/test_functions.py::FunctionTests::test_namedtuple_subclass
test/dynamo/test_functions.py::FunctionTests::test_generic_namedtuple_subclass
test/dynamo/test_functions.py::DefaultsTests::test_udf_namedtuple
test/dynamo/test_dicts.py::DictTests::test_dict_namedtuple
test/dynamo/test_dicts.py::DictTests::test_lazy_key_non_const_guarding
test/dynamo/test_modules.py::NNModuleTests::test_lazy_module7
test/dynamo/test_subclasses.py::SubclassTests::test_subclass_constructor_proxying
PYTORCH_TEST_WITH_DYNAMO=1 test/dynamo/cpython/3_13/test_collections.py::TestNamedTuple

morrison-turnansky · 2025-11-13T16:32:28Z

@isuruf addressed failing tests, we should be good now

torch/_dynamo/variables/builtin.py

StrongerXi

This looks great! Could you please keep NamedTupleVariable inside lists.py though, for 2 reasons:

To minimize changes and make the diff clearer.
We already have more non-primitive container classes like deque and torch.Size emulated inside lists.py, so it makes sense to keep named tuple there as well.

I think you can avoid circular dependency by moving this to within where it's used:

pytorch/torch/_dynamo/variables/user_defined.py

Line 99 in 780e325

from .lists import SizeVariable

pytorch/torch/_dynamo/variables/user_defined.py

Lines 744 to 747 in 780e325

    
           elif self.value is torch.Size: 
        
               # This simulates `THPSize_pynew`, the C impl for `Size.__new__`. 
        
               tup = variables.BuiltinVariable(tuple).call_function(tx, args, kwargs) 
        
               return SizeVariable(tup.items)

torch/_dynamo/variables/lists.py

morrison-turnansky · 2025-11-18T13:43:20Z

@StrongerXi I moved NamedTupleVariable back to list. You'll see some additional changes to fix circular dependencies besides what you listed. In addition, since typing errors in user_defined.py is ignored, I had to make some linting changes. Everything else is more or less the same.

torch/_dynamo/variables/lists.py

torch/_dynamo/variables/user_defined.py

torch/_dynamo/variables/lists.py

guilhermeleobas

One a few minor comments but the code looks good to me. Thanks for the contribution @morrison-turnansky

StrongerXi

Dropped a nit on docs, otherwise LGTM! Please address Guilherme's comments too. Thank you for this clean up!

torch/_dynamo/variables/lists.py

morrison-turnansky · 2025-11-19T20:20:15Z

@pytorchbot merge

pytorchmergebot · 2025-11-19T20:25:47Z

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

pytorchmergebot · 2025-11-19T20:26:34Z

Merge failed

Reason: 1 mandatory check(s) failed. The first few are:

pull / cuda12.8-py3.10-gcc11-sm75 / build

Dig deeper by viewing the failures on hud

Details for Dev Infra team

Raised by workflow job

Failing merge rule: Core Maintainers

StrongerXi · 2025-11-20T00:56:51Z

@pytorchbot merge -f "unrelated build error"

morrison-turnansky · 2026-01-12T16:57:26Z

@pytorchbot merge

pytorch-bot · 2026-01-12T16:57:31Z

Pull workflow has not been scheduled for the PR yet. It could be because author doesn't have permissions to run those or skip-checks keywords were added to PR/commits, aborting merge. Please get/give approval for the workflows and/or remove skip ci decorators before next merge attempt. If you think this is a mistake, please contact PyTorch Dev Infra.

morrison-turnansky · 2026-01-12T17:09:38Z

@pytorchbot merge

pytorch-bot · 2026-01-12T17:09:43Z

Pull workflow has not been scheduled for the PR yet. It could be because author doesn't have permissions to run those or skip-checks keywords were added to PR/commits, aborting merge. Please get/give approval for the workflows and/or remove skip ci decorators before next merge attempt. If you think this is a mistake, please contact PyTorch Dev Infra.

morrison-turnansky · 2026-01-12T17:19:17Z

@pytorchbot merge

pytorch-bot · 2026-01-12T17:19:23Z

Pull workflow has not been scheduled for the PR yet. It could be because author doesn't have permissions to run those or skip-checks keywords were added to PR/commits, aborting merge. Please get/give approval for the workflows and/or remove skip ci decorators before next merge attempt. If you think this is a mistake, please contact PyTorch Dev Infra.

morrison-turnansky · 2026-01-12T17:23:02Z

@pytorchbot merge

pytorch-bot · 2026-01-12T17:23:12Z

Pull workflow has not been scheduled for the PR yet. It could be because author doesn't have permissions to run those or skip-checks keywords were added to PR/commits, aborting merge. Please get/give approval for the workflows and/or remove skip ci decorators before next merge attempt. If you think this is a mistake, please contact PyTorch Dev Infra.

skpark-rh · 2026-01-12T17:37:00Z

@pytorchbot merge

pytorchmergebot · 2026-01-12T17:39:01Z

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

pytorchmergebot · 2026-01-12T22:52:28Z

Merge failed

Reason: 1 jobs have failed, first few of them are: trunk / linux-jammy-rocm-py3.10 / test (distributed, 3, 3, linux.rocm.gpu.gfx942.4)

Details for Dev Infra team

Raised by workflow job

skpark-rh · 2026-01-13T18:39:58Z

@pytorch merge -i

skpark-rh · 2026-01-13T18:40:35Z

@pytorchbot merge -i

pytorchmergebot · 2026-01-13T18:42:30Z

Merge started

Your change will be merged while ignoring the following 1 checks: trunk / linux-jammy-rocm-py3.10 / test (distributed, 2, 3, linux.rocm.gpu.gfx942.4)

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

@StrongerXi

…Variable (pytorch#167468) Continuation of work from previous PR, see link for context pytorch#161645 (comment) I think this PR is a step in that direction. There is probably some room for simplification. At a high level, the new class NamedTupleVariable handles methods that branch on structseq or the more dynamic subclasses of namedtuple, and falls back to UserDefinedTupleVariable otherwise. Please let me know what you think. @StrongerXi Pull Request resolved: pytorch#167468 Approved by: https://github.com/Lucaskabela

pytorch-bot bot added the module: dynamo label Nov 10, 2025

pytorchbot added the open source label Nov 10, 2025

morrison-turnansky force-pushed the named-tuple branch 2 times, most recently from 597d115 to 768d30d Compare November 11, 2025 12:51

morrison-turnansky changed the title ~~Named tuple~~ Change NamedTupleVariable implementation to subclass UserDefinedTupleVariable Nov 12, 2025

isuruf requested a review from guilhermeleobas November 12, 2025 20:01

soulitzer added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Nov 13, 2025

guilhermeleobas reviewed Nov 14, 2025

View reviewed changes

torch/_dynamo/variables/builtin.py Outdated Show resolved Hide resolved

morrison-turnansky requested a review from guilhermeleobas November 14, 2025 15:01

guilhermeleobas added the topic: not user facing topic category label Nov 14, 2025

guilhermeleobas requested a review from StrongerXi November 14, 2025 18:03

StrongerXi reviewed Nov 14, 2025

View reviewed changes

torch/_dynamo/variables/lists.py Show resolved Hide resolved

guilhermeleobas reviewed Nov 18, 2025

View reviewed changes

torch/_dynamo/variables/lists.py Show resolved Hide resolved

guilhermeleobas reviewed Nov 18, 2025

View reviewed changes

torch/_dynamo/variables/user_defined.py Outdated Show resolved Hide resolved

guilhermeleobas reviewed Nov 18, 2025

View reviewed changes

torch/_dynamo/variables/lists.py Outdated Show resolved Hide resolved

guilhermeleobas previously approved these changes Nov 18, 2025

View reviewed changes

StrongerXi previously approved these changes Nov 18, 2025

View reviewed changes

torch/_dynamo/variables/lists.py Outdated Show resolved Hide resolved

mlazos previously approved these changes Nov 19, 2025

View reviewed changes

pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Nov 19, 2025

pytorchmergebot added the merging label Nov 19, 2025

pytorchmergebot removed the merging label Nov 19, 2025

pytorchmergebot added the merging label Nov 20, 2025

morrison-turnansky added 6 commits January 12, 2026 13:13

reviwer comments

aa958d7

linting

acfc305

ci fix

cfcf5ec

tests passing

5e1e7f4

linting

4db7001

fixed vllm ci issue, added pytorch test to avoid issue in future

992fe52

morrison-turnansky force-pushed the named-tuple branch from 3d026d0 to 992fe52 Compare January 12, 2026 13:52

pytorch-bot bot removed the ciflow/trunk Trigger trunk jobs on your pull request label Jan 12, 2026

pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Jan 12, 2026

pytorchmergebot added the merging label Jan 12, 2026

pytorchmergebot removed the merging label Jan 12, 2026

pytorchmergebot added the merging label Jan 13, 2026

pytorchmergebot closed this in 37a8c7d Jan 13, 2026

pytorchmergebot removed the merging label Jan 13, 2026

	elif self.value is torch.Size:
	# This simulates `THPSize_pynew`, the C impl for `Size.__new__`.
	tup = variables.BuiltinVariable(tuple).call_function(tx, args, kwargs)
	return SizeVariable(tup.items)

Conversation

morrison-turnansky commented Nov 10, 2025 • edited by pytorch-bot bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

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

✅ You can merge normally! (1 Unrelated Failure)

Uh oh!

morrison-turnansky commented Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

morrison-turnansky commented Nov 13, 2025

Uh oh!

Uh oh!

StrongerXi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

morrison-turnansky commented Nov 18, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

guilhermeleobas left a comment

Choose a reason for hiding this comment

Uh oh!

StrongerXi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

morrison-turnansky commented Nov 19, 2025

Uh oh!

pytorchmergebot commented Nov 19, 2025

Merge started

Uh oh!

pytorchmergebot commented Nov 19, 2025

Merge failed

Uh oh!

StrongerXi commented Nov 20, 2025

Uh oh!

morrison-turnansky commented Jan 12, 2026

Uh oh!

pytorch-bot bot commented Jan 12, 2026

Uh oh!

morrison-turnansky commented Jan 12, 2026

Uh oh!

pytorch-bot bot commented Jan 12, 2026

Uh oh!

morrison-turnansky commented Jan 12, 2026

Uh oh!

pytorch-bot bot commented Jan 12, 2026

Uh oh!

morrison-turnansky commented Jan 12, 2026

Uh oh!

pytorch-bot bot commented Jan 12, 2026

Uh oh!

skpark-rh commented Jan 12, 2026

Uh oh!

pytorchmergebot commented Jan 12, 2026

Merge started

Uh oh!

pytorchmergebot commented Jan 12, 2026

Merge failed

Uh oh!

skpark-rh commented Jan 13, 2026

Uh oh!

skpark-rh commented Jan 13, 2026

Uh oh!

pytorchmergebot commented Jan 13, 2026

Merge started

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

morrison-turnansky commented Nov 10, 2025 •

edited by pytorch-bot bot

Loading

pytorch-bot bot commented Nov 10, 2025 •

edited

Loading

morrison-turnansky commented Nov 10, 2025 •

edited

Loading