-
Notifications
You must be signed in to change notification settings - Fork 105
Fixing Ianvs Example Execution Patterns Through Standardization (Proposal) #328
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
|
Welcome @PRIYANSHU2026! It looks like this is your first PR to kubeedge/ianvs 🎉 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: PRIYANSHU2026 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Summary of ChangesHello @PRIYANSHU2026, 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 detailed proposal and framework aimed at resolving systematic execution failures observed in Ianvs examples. The core objective is to standardize how examples handle paths, environments, and dependencies, ensuring they are consistently runnable, reproducible, and compatible with CI/CD pipelines. By addressing issues like hard-coded paths, environment-specific assumptions, and fragile dependency management, the PR seeks to significantly improve user experience and maintainability across the project's benchmarking examples. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
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 proposal to standardize Ianvs examples, addressing critical issues like hard-coded paths, environment dependencies, and fragile dependency management. The proposal is well-structured, clearly outlining the current state, universal goals, design principles, and an implementation roadmap. The use of llm_simple_qa as a case study effectively demonstrates the proposed fixes. The initiative to improve reproducibility, CI/CD compatibility, and user experience is highly commendable. My feedback focuses on minor inconsistencies in figure numbering and potential improvements for robustness and clarity in the illustrative code snippets.
| - Test imports and basic initialization | ||
| - Run minimal smoke tests where feasible | ||
|  | ||
| *Figure 4: Architectural transformation from fragile, environment-dependent examples to robust, portable implementations through interface abstraction and standardization* |
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.
There's an inconsistency in the figure numbering within the text and the image filenames. For example, Figure3.png is labeled as Figure 4 here, Figure5.png is labeled as Figure 6 (line 131), Figure 6.png is labeled as Figure 3 (line 164), and Figure 7.png is labeled as Figure 5 (line 259). Please ensure the figure numbers in the text consistently match the image filenames or are sequentially correct.
| def resolve_example_path(relative_path, config_file=None): | ||
| """Resolve paths relative to example root or config file.""" | ||
| if config_file: | ||
| base = os.path.dirname(os.path.abspath(config_file)) |
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.
In the resolve_example_path function, if config_file is None, os.path.abspath(config_file) will raise a TypeError. While this is an illustrative snippet, it's good to ensure robustness. Consider adding a check to ensure config_file is not None before passing it to os.path.abspath.
| base = os.path.dirname(os.path.abspath(config_file)) | |
| base = os.path.dirname(os.path.abspath(config_file)) if config_file else None |
| base = os.path.dirname(os.path.abspath(config_file)) | ||
| else: | ||
| # Walk up from current file to find example root | ||
| base = find_example_root(__file__) |
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.
| example: | ||
| workspace: "${IANVS_WORKSPACE:-./workspace}" | ||
| dataset_dir: "${DATASET_DIR:-./dataset}" | ||
| model_cache: "${MODEL_CACHE:-~/.cache/ianvs/models}" |
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.
Using ~/.cache/ianvs/models with ${MODEL_CACHE:-...} might not reliably expand the tilde (~) to the user's home directory across all environments or YAML parsers. It's generally safer and more explicit to use $HOME for home directory references, e.g., ${MODEL_CACHE:-$HOME/.cache/ianvs/models}.
| model_cache: "${MODEL_CACHE:-~/.cache/ianvs/models}" | |
| model_cache: "${MODEL_CACHE:-$HOME/.cache/ianvs/models}" |
Proposal :- Fix Example Execution Patterns – Standardize Paths, Environments & Dependencies
Respected @MooreZheng @AryanNanda17
Hi, I’m Priyanshu Tiwari, an LFX 2026 Term 1 applicant and open-source contributor with prior experience in GSoC 2025 and Mifos Summer of Code 2025, working across cloud-native systems, AI, and reproducible ML workflows.
Mentorship Experience
Research Background (14+ papers, multidisciplinary – IEEE / Springer / E3S)
Portfolio: https://priyanshu-2026-github-io.vercel.app/
Summary
This PR introduces a unified execution fix framework to eliminate common failures across Ianvs examples. Using
llm_simple_qaas a reference, it standardizes path resolution, environment handling, and dependency management so examples work out of the box.Key Fixes
${IANVS_WORKSPACE:-./workspace}JsonlDataLoaderResults
✅ Fresh-environment execution
✅ CPU fallback for GPU-only examples
✅ Cross-platform support
✅ CI passing
References
Happy to iterate further based on maintainer feedback and align with Ianvs best practices 🚀