Skip to content

Allow function bodies to (initially) contain duplicate nodes#637

Merged
inducer merged 2 commits intoinducer:mainfrom
majosm:allow-duplicates-in-function-body
Feb 24, 2026
Merged

Allow function bodies to (initially) contain duplicate nodes#637
inducer merged 2 commits intoinducer:mainfrom
majosm:allow-duplicates-in-function-body

Conversation

@majosm
Copy link
Collaborator

@majosm majosm commented Feb 19, 2026

FunctionDefinition uses InputGatherer when searching the DAG for its argument placeholders, which gives an error if the function body initially contains any duplicate nodes. This relaxes that requirement by adding a ListOfInputsGatherer that caches by id instead. (Note: the argument placeholders themselves are still not allowed to be duplicated, but AFAIK that shouldn't ever happen.)

@majosm majosm force-pushed the allow-duplicates-in-function-body branch from 6bcc82c to 80fe016 Compare February 23, 2026 16:24
…n code

InputGatherer is too strict, as it will error on any duplicated nodes in the function body
@inducer inducer force-pushed the allow-duplicates-in-function-body branch from 80fe016 to 1e3bacb Compare February 23, 2026 16:52
@majosm majosm force-pushed the allow-duplicates-in-function-body branch from 1e3bacb to 9eec7a5 Compare February 23, 2026 16:52
@inducer
Copy link
Owner

inducer commented Feb 23, 2026

Could you speak a bit about the rationale for this change?

@majosm
Copy link
Collaborator Author

majosm commented Feb 23, 2026

Could you speak a bit about the rationale for this change?

Yeah. Duplicates can arise from ordinary math operations on the application side; it shouldn't be an error for them to be present initially. We just remove them at the start of DAG transforming. But InputGatherer (called from FunctionDefinition.__call__, i.e. during DAG assembly) errors if duplicates are present. This prevents that error.

@inducer inducer merged commit a39e5b8 into inducer:main Feb 24, 2026
10 checks passed
@inducer
Copy link
Owner

inducer commented Feb 24, 2026

Makes sense, thx!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants