-
Notifications
You must be signed in to change notification settings - Fork 45
Feature/graph net bench #541
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
base: develop
Are you sure you want to change the base?
Feature/graph net bench #541
Conversation
…12/GraphNet into feature/graph_net_bench merge new code
…12/GraphNet into feature/graph_net_bench merge new code
|
Thanks for your contribution! |
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.
Pull request overview
This PR adds remote execution capabilities for graph network benchmarking by implementing a gRPC-based remote executor. The feature allows test execution on a remote server via the SampleRemoteExecutor class, with corresponding client and server implementations.
Changes:
- Implements SampleRemoteExecutor with gRPC client functionality for compressing/sending model directories and receiving results
- Adds test_remote_reference_device.py client to execute reference device tests remotely
- Updates gRPC server to handle model execution requests with tarball compression/decompression
- Integrates remote testing into the subgraph decomposition workflow with new test module configuration
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 23 comments.
Show a summary per file
| File | Description |
|---|---|
| graph_net/torch/test_remote_reference_device.py | New client for executing reference device tests on remote servers via gRPC |
| graph_net/graph_net_bench/sample_remote_executor.py | Complete implementation of remote executor with compression and gRPC communication |
| graph_net/graph_net_bench/grpc/server.py | Server implementation for handling remote execution requests with subprocess execution |
| graph_net/graph_net_bench/grpc/sample_remote_executor_test.py | Test/CLI client for the remote executor (has critical bugs) |
| graph_net/test/subgraph_decompose_and_evaluation_step_test.sh | Adds configuration for test_remote_reference_device module |
| graph_net/subgraph_decompose_and_evaluation_step.py | Integrates remote reference device testing into task scheduler |
| graph_net/graph_net_bench/grpc/message.proto | Adds random_seed field to ExecutionRequest |
| graph_net/graph_net_bench/grpc/message_pb2.py | Regenerated protobuf Python code with updated schema |
| graph_net/graph_net_bench/grpc/message_pb2_grpc.py | Regenerated gRPC Python code with formatting changes |
| graph_net/graph_net_bench/grpc/client.py | Removed old example client code |
| graph_net/graph_net_bench/grpc/init.py | Added package marker with comment |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| with tarfile.open(fileobj=buffer, mode="r:gz") as tar: | ||
| for member in tar.getmembers(): | ||
| if member.isfile(): | ||
| file_content = tar.extractfile(member).read() |
Copilot
AI
Jan 12, 2026
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.
The tar.extractfile(member).read() doesn't explicitly close the file object returned by extractfile(). While it may be garbage collected, it's better practice to use a context manager or explicitly close the file to avoid resource leaks, especially in a server that may handle many requests.
| file_content = tar.extractfile(member).read() | |
| extracted_file = tar.extractfile(member) | |
| if extracted_file is None: | |
| continue | |
| with extracted_file as f: | |
| file_content = f.read() |
| def __call__(self, model_path: str, random_seed: int) -> dict: | ||
|
|
Copilot
AI
Jan 12, 2026
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.
The method signature indicates it returns 'dict' but lacks documentation about the structure of the dictionary. Add a docstring explaining that it returns a dict mapping filenames (str) to file contents (bytes) to help users understand the return value structure.
| def __call__(self, model_path: str, random_seed: int) -> dict: | |
| def __call__(self, model_path: str, random_seed: int) -> dict: | |
| """ | |
| Execute the remote model with the given seed and return its output files. | |
| Returns: | |
| dict: A mapping from output filenames (str) to their file contents (bytes). | |
| """ |
PR Category
Feature Enhancement
Description
Add SampleRemoteExecutor