-
Notifications
You must be signed in to change notification settings - Fork 4
Add helper functions for distributed micro benchmarks #88
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: main
Are you sure you want to change the base?
Conversation
Summary of ChangesHello @PranjalC100, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a suite of Python helper modules designed to facilitate and automate distributed micro-benchmarking workflows on Google Cloud Platform. The new functionalities cover the entire lifecycle of a distributed benchmark, from setting up test environments and distributing workloads to collecting, aggregating, and reporting results. This significantly enhances the framework's capability to run complex, multi-VM benchmarks efficiently and reliably. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request introduces a comprehensive set of helper modules for running distributed micro-benchmarks. The code is well-structured, with clear separation of concerns for interacting with gcloud/GCS, generating test jobs, aggregating results, and generating reports.
I've identified several issues, including a critical typo in an exception name, a few high-severity bugs that could lead to crashes or incorrect results (potential division by zero, incorrect metric aggregation, and malformed CSV output), and a potential file creation issue with unsanitized input. I've also included several medium-severity suggestions to improve error handling, code consistency, and adherence to Python style conventions (PEP 8), such as adding missing newlines at the end of files and improving documentation.
Overall, this is a solid foundation for the benchmarking framework. Addressing the identified issues will significantly improve its robustness and maintainability.
| if os.path.exists(test_dir): | ||
| metrics = parse_test_results(test_dir, test_info, mode) | ||
| all_metrics[test_key] = metrics | ||
| successful_vms += 1 |
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 successful_vms counter is incremented for each successfully processed test, not for each VM. This will lead to an incorrect summary message at the end (e.g., "Successfully aggregated results from 100/10 VMs"). This counter should be incremented only once per VM, likely after its manifest has been successfully downloaded and opened.
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.
WIll have a look at this later.
No description provided.