Skip to content

Conversation

[ghstack-poisoned]
@pytorch-bot
Copy link

pytorch-bot bot commented Jul 4, 2025

🔗 Helpful Links

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

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 be05e6c with merge base c808af5 (image):

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

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.

[ghstack-poisoned]
@bobrenjc93 bobrenjc93 requested a review from aorenste July 4, 2025 19:57
@bobrenjc93 bobrenjc93 added topic: not user facing topic category ciflow/trunk Trigger trunk jobs on your pull request labels Jul 4, 2025
@bobrenjc93 bobrenjc93 marked this pull request as ready for review July 4, 2025 19:57
self._fast_output_code = fast_output_code
self._optimized_output_code = None
self._progression_futures = list(progression_futures)
self._progression_futures = deque(progression_futures)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Above (github won't let me comment on unchanged lines):

    progression_futures: Sequence[Future[_WireProtocolPickledOutput]],

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Problem is then mypy complains about no popleft

  Error (MYPY) [attr-defined]
    "Sequence[Future[_WireProtocolPickledOutput]]" has no attribute "popleft"

        268  |
        269  |        # Clear earlier progression futures to free memory
        270  |        for _ in range(stage_index + 1):
    >>> 271  |            self._progression_futures.popleft()
        272  |
        273  |    @override
        274  |    def post_compile(

Copy link
Contributor

@aorenste aorenste Jul 5, 2025

Choose a reason for hiding this comment

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

I think you misunderstood. I meant the init signature should take Sequence. The instance member should still be deque. (I can see that what I wrote wasn't clear)

[ghstack-poisoned]
[ghstack-poisoned]
@pytorchmergebot
Copy link
Collaborator

Starting merge as part of PR stack under #157650

pytorchmergebot pushed a commit that referenced this pull request Jul 5, 2025
followup from #157305 where
@aorenste correctly suggested clearing callback. this refactor
introduces a new dataclass so we don't need to check nullability for
each field

Pull Request resolved: #157619
Approved by: https://github.com/aorenste
ghstack dependencies: #157305, #157614
pytorchmergebot pushed a commit that referenced this pull request Jul 5, 2025
@github-actions github-actions bot deleted the gh/bobrenjc93/511/head branch August 5, 2025 02:19
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.

4 participants