Skip to content

[Dynamo][Better Engineering] Add typing for comptime, cache, and convert_frame #158379

Closed
Lucaskabela wants to merge 1 commit intomainfrom
lucaskabela/typing_cache_files
Closed

[Dynamo][Better Engineering] Add typing for comptime, cache, and convert_frame #158379
Lucaskabela wants to merge 1 commit intomainfrom
lucaskabela/typing_cache_files

Conversation

@Lucaskabela
Copy link
Contributor

@Lucaskabela Lucaskabela commented Jul 15, 2025

As part of better engineering week, we would like to improve out type support to improve dev experience in dynamo

This PR adds strict typing support to a critical tracing point for dynamo, primarily forcomptime.py but also cache_size.py and convert_frame.py.

Running

mypy torch/_dynamo/comptime.py torch/_dynamo/cache_size.py torch/_dynamo/convert_frame.py --linecount-report /tmp/coverage_log
-------- Lines Unannotated Lines Total % lines covered Funcs Unannotated Funcs Total % funcs covered
Main 1837 2215 82.93% 45 82 54.88%
This PR 2230 2230 100.00% 82 82 100.00%
Delta +393 +15 +17.07% +37 0 +45.12%

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

@pytorch-bot
Copy link

pytorch-bot bot commented Jul 15, 2025

🔗 Helpful Links

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

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

✅ You can merge normally! (2 Unrelated Failures)

As of commit 42eb33f with merge base 16b21fa (image):

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

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

UNSTABLE - The following job is marked as unstable, possibly due to flakiness on trunk:

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



@TorchPatcher.suppress_torch_distributed_warnings
@TorchPatcher.suppress_torch_distributed_warnings # type: ignore[misc]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This error should go away once we land #158276 but we need to add to avoid race conditions for now

@Lucaskabela
Copy link
Contributor Author

@pytorchbot label "topic: not user facing"

@pytorch-bot pytorch-bot bot added the topic: not user facing topic category label Jul 15, 2025
@Lucaskabela Lucaskabela changed the title [Better Engineering] Add typing for comptime, cache, and convert_frame [Dynamo][Better Engineering] Add typing for comptime, cache, and convert_frame Jul 15, 2025
@Lucaskabela Lucaskabela marked this pull request as ready for review July 15, 2025 23:25
@Lucaskabela Lucaskabela force-pushed the lucaskabela/typing_cache_files branch from 930dda2 to 42eb33f Compare July 17, 2025 16:39
constraints: Optional[list[Constraint]] = None,
**extra_kwargs: Any,
) -> Callable[[tuple[Any, Any]], ExportResult]:
) -> Callable[..., ExportResult]:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

tuple[Any, Any] isn't quite right since truly this is args and kwargs; for now we can use the less restrictive ... here

Copy link
Contributor

Choose a reason for hiding this comment

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

Can't you do tuple[...] ?

@Lucaskabela
Copy link
Contributor Author

@pytorchmergebot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Jul 17, 2025
@pytorchmergebot
Copy link
Collaborator

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

@github-actions github-actions bot deleted the lucaskabela/typing_cache_files branch August 17, 2025 02:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants