-
Notifications
You must be signed in to change notification settings - Fork 661
[BugFix] dummy import some ops #5192
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,77 +1,107 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Copyright (c) 2025 PaddlePaddle Authors. All Rights Reserved. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Copyright (c) 2025 PaddlePaddle Authors. All Rights Reserved. | |
| # Copyright (c) 2025 PaddlePaddle Authors. All Rights Reserved. |
Copilot
AI
Nov 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon in license header: The Apache License header comment should end with ");" not just ")".
| # Licensed under the Apache License, Version 2.0 (the "License" | |
| # Licensed under the Apache License, Version 2.0 (the "License"); |
Copilot
AI
Nov 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spelling error: "UNIMPLENENTED" should be "UNIMPLEMENTED".
| raise RuntimeError("XPU get_data_ptr_ipc UNIMPLENENTED!") | |
| def ipc_sent_key_value_cache_by_remote_ptr(*args, **kwargs): | |
| raise RuntimeError("XPU ipc_sent_key_value_cache_by_remote_ptr UNIMPLENENTED") | |
| def ipc_sent_key_value_cache_by_remote_ptr_block_sync(*args, **kwargs): | |
| raise RuntimeError("XPU No ipc_sent_key_value_cache_by_remote_ptr UNIMPLENENTED") | |
| raise RuntimeError("XPU get_data_ptr_ipc UNIMPLEMENTED!") | |
| def ipc_sent_key_value_cache_by_remote_ptr(*args, **kwargs): | |
| raise RuntimeError("XPU ipc_sent_key_value_cache_by_remote_ptr UNIMPLEMENTED") | |
| def ipc_sent_key_value_cache_by_remote_ptr_block_sync(*args, **kwargs): | |
| raise RuntimeError("XPU No ipc_sent_key_value_cache_by_remote_ptr UNIMPLEMENTED") |
Copilot
AI
Nov 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spelling error: "UNIMPLENENTED" should be "UNIMPLEMENTED".
| raise RuntimeError("XPU get_data_ptr_ipc UNIMPLENENTED!") | |
| def ipc_sent_key_value_cache_by_remote_ptr(*args, **kwargs): | |
| raise RuntimeError("XPU ipc_sent_key_value_cache_by_remote_ptr UNIMPLENENTED") | |
| def ipc_sent_key_value_cache_by_remote_ptr_block_sync(*args, **kwargs): | |
| raise RuntimeError("XPU No ipc_sent_key_value_cache_by_remote_ptr UNIMPLENENTED") | |
| raise RuntimeError("XPU get_data_ptr_ipc UNIMPLEMENTED!") | |
| def ipc_sent_key_value_cache_by_remote_ptr(*args, **kwargs): | |
| raise RuntimeError("XPU ipc_sent_key_value_cache_by_remote_ptr UNIMPLEMENTED") | |
| def ipc_sent_key_value_cache_by_remote_ptr_block_sync(*args, **kwargs): | |
| raise RuntimeError("XPU No ipc_sent_key_value_cache_by_remote_ptr UNIMPLEMENTED") |
Copilot
AI
Nov 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spelling error: "UNIMPLENENTED" should be "UNIMPLEMENTED".
| raise RuntimeError("XPU get_data_ptr_ipc UNIMPLENENTED!") | |
| def ipc_sent_key_value_cache_by_remote_ptr(*args, **kwargs): | |
| raise RuntimeError("XPU ipc_sent_key_value_cache_by_remote_ptr UNIMPLENENTED") | |
| def ipc_sent_key_value_cache_by_remote_ptr_block_sync(*args, **kwargs): | |
| raise RuntimeError("XPU No ipc_sent_key_value_cache_by_remote_ptr UNIMPLENENTED") | |
| raise RuntimeError("XPU get_data_ptr_ipc UNIMPLEMENTED!") | |
| def ipc_sent_key_value_cache_by_remote_ptr(*args, **kwargs): | |
| raise RuntimeError("XPU ipc_sent_key_value_cache_by_remote_ptr UNIMPLEMENTED") | |
| def ipc_sent_key_value_cache_by_remote_ptr_block_sync(*args, **kwargs): | |
| raise RuntimeError("XPU No ipc_sent_key_value_cache_by_remote_ptr UNIMPLEMENTED") |
Copilot
AI
Nov 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bare except clause is a bad practice: Using a bare except: catches all exceptions including SystemExit, KeyboardInterrupt, and other exceptions that should typically propagate. This can mask unexpected errors and make debugging difficult.
Consider catching specific exceptions instead:
except (ImportError, ModuleNotFoundError, AttributeError) as e:
# Log the error for debugging
# Then set fallback valuesThis would catch import-related errors while allowing critical exceptions to propagate properly.
Copilot
AI
Nov 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Silent failure with no logging: The except block silently sets all functions to None without any logging or warning. This makes it extremely difficult to diagnose issues when imports fail.
Consider adding logging to help with debugging:
except (ImportError, ModuleNotFoundError) as e:
import logging
logger = logging.getLogger(__name__)
logger.warning(f"Failed to import platform-specific ops: {e}. Setting fallback values.")
# Set None valuesThis would help developers and users understand why operations are unavailable.
Copilot
AI
Nov 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting functions to None can cause AttributeError at runtime: When these functions are set to None in the except block, any code that tries to call them (e.g., cuda_host_alloc()) will fail with TypeError: 'NoneType' object is not callable. This defers error detection to runtime rather than providing clear feedback at import time.
Consider either:
- Raising a clear error at import time if these operations are critical
- Creating stub functions that raise informative errors when called:
def _create_stub(name):
def stub(*args, **kwargs):
raise RuntimeError(f"{name} is not available: platform-specific ops failed to import")
return stub
# Then in the except block:
cuda_host_alloc = _create_stub("cuda_host_alloc")
cuda_host_free = _create_stub("cuda_host_free")
# etc.This provides better error messages when the functions are actually used.
| cuda_host_alloc = None | |
| cuda_host_free = None | |
| set_data_ipc = None | |
| share_external_data_ = None | |
| swap_cache_all_layers = None | |
| unset_data_ipc = None | |
| set_device = None | |
| memory_allocated = None | |
| get_output_kv_signal = None | |
| get_data_ptr_ipc = None | |
| ipc_sent_key_value_cache_by_remote_ptr = None | |
| ipc_sent_key_value_cache_by_remote_ptr_block_sync = None | |
| get_peer_mem_addr = None | |
| get_all_visible_devices = None | |
| def _create_stub(name): | |
| def stub(*args, **kwargs): | |
| raise RuntimeError(f"{name} is not available: platform-specific ops failed to import") | |
| return stub | |
| cuda_host_alloc = _create_stub("cuda_host_alloc") | |
| cuda_host_free = _create_stub("cuda_host_free") | |
| set_data_ipc = _create_stub("set_data_ipc") | |
| share_external_data_ = _create_stub("share_external_data_") | |
| swap_cache_all_layers = _create_stub("swap_cache_all_layers") | |
| unset_data_ipc = _create_stub("unset_data_ipc") | |
| set_device = _create_stub("set_device") | |
| memory_allocated = _create_stub("memory_allocated") | |
| get_output_kv_signal = _create_stub("get_output_kv_signal") | |
| get_data_ptr_ipc = _create_stub("get_data_ptr_ipc") | |
| ipc_sent_key_value_cache_by_remote_ptr = _create_stub("ipc_sent_key_value_cache_by_remote_ptr") | |
| ipc_sent_key_value_cache_by_remote_ptr_block_sync = _create_stub("ipc_sent_key_value_cache_by_remote_ptr_block_sync") | |
| get_peer_mem_addr = _create_stub("get_peer_mem_addr") | |
| get_all_visible_devices = _create_stub("get_all_visible_devices") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Title Missing Required Tag: The PR title "dummy import fd" does not follow the required format of [CLASS]Title. According to the coding guidelines and checklist, it should include a tag such as [BugFix], [Feature], [Optimization], etc. For example: "[BugFix] Add fallback handling for import errors" or "[Feature] Add error handling for platform-specific imports".