Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix hpu destructors flow and remove finish_measurements #379

Merged
merged 2 commits into from
Dec 9, 2024

Conversation

nirda7
Copy link

@nirda7 nirda7 commented Oct 10, 2024

Fix hpu related destructors flow, avoiding multiple calls to the same shutdown function.
Remove the use of the finish_measurements function.

Tiefen-boop
Tiefen-boop previously approved these changes Oct 13, 2024
Copy link

@kzawora-intel kzawora-intel left a comment

Choose a reason for hiding this comment

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

LGTM overall, but let's add a guard against AttributeError in the destructor by using getattr (in case executor's destructor gets called before _init_executor())

vllm/executor/ray_hpu_executor.py Outdated Show resolved Hide resolved
vllm/executor/hpu_executor.py Outdated Show resolved Hide resolved
@nirda7 nirda7 force-pushed the fix_destructors_flow branch from b2e342c to ba61352 Compare October 24, 2024 07:22
@nirda7 nirda7 force-pushed the fix_destructors_flow branch 10 times, most recently from 9c9d748 to adbfb47 Compare November 26, 2024 09:37
@nirda7 nirda7 force-pushed the fix_destructors_flow branch 2 times, most recently from a57df85 to 5b1658a Compare November 28, 2024 14:38
@nirda7 nirda7 force-pushed the fix_destructors_flow branch 3 times, most recently from d539a6c to 6454f3f Compare December 5, 2024 13:11
@nirda7 nirda7 requested a review from Tiefen-boop December 5, 2024 13:24
@nirda7 nirda7 dismissed Tiefen-boop’s stale review December 5, 2024 13:25

New code - need to review it again

@nirda7 nirda7 force-pushed the fix_destructors_flow branch from 6454f3f to 029f9f9 Compare December 8, 2024 10:08
@nirda7 nirda7 force-pushed the fix_destructors_flow branch from 029f9f9 to 592121e Compare December 9, 2024 12:51
Copy link

@afierka-intel afierka-intel left a comment

Choose a reason for hiding this comment

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

LGTM

@michalkuligowski michalkuligowski merged commit db68690 into habana_main Dec 9, 2024
9 checks passed
@michalkuligowski michalkuligowski deleted the fix_destructors_flow branch December 9, 2024 13:36
afierka-intel pushed a commit that referenced this pull request Dec 11, 2024
Fix hpu related destructors flow, avoiding multiple calls to the same
shutdown function.
Remove the use of the finish_measurements function.
kzawora-intel pushed a commit that referenced this pull request Dec 12, 2024
Cherry-pick of existing fix from habana_main.

Fix hpu related destructors flow, avoiding multiple calls to the same
shutdown function.
Remove the use of the finish_measurements function.

---------

Co-authored-by: Nir David <[email protected]>
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.

5 participants