fix: resolve missing dataset and config issues in compression benchmark.#542
fix: resolve missing dataset and config issues in compression benchmark.#542Fahmid-Arman wants to merge 1 commit into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Fahmid-Arman 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 |
There was a problem hiding this comment.
Code Review
This pull request bundles a minimal dataset directly with the single-task compression benchmark suite, updates the test environment configuration to use relative paths pointing to this dataset, and simplifies the setup instructions in the README. Feedback highlights path inconsistencies in the README's model download commands and a discrepancy between the execution directory assumed by the relative paths in the configuration and the commands documented in the README.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| ```bash | ||
| mkdir -p /ianvs/models/qwen | ||
| wget -c -O ianvs/models/qwen/qwen_1_5_0_5b.gguf [https://huggingface.co/Qwen/Qwen1.5-0.5B-Chat-GGUF/resolve/main/qwen1_5-0_5b-chat-q4_k_m.gguf](https://huggingface.co/Qwen/Qwen1.5-0.5B-Chat-GGUF/resolve/main/qwen1_5-0_5b-chat-q4_k_m.gguf) | ||
| wget -c -O ianvs/models/qwen/qwen_1_5_0_5b.gguf https://huggingface.co/Qwen/Qwen1.5-0.5B-Chat-GGUF/resolve/main/qwen1_5-0_5b-chat-q4_k_m.gguf |
There was a problem hiding this comment.
There is a path inconsistency between the directory creation command and the wget download command:
mkdir -p /ianvs/models/qwen(on line 30) uses an absolute path starting with/, which might fail due to permission issues and is likely not the intended location.wget -c -O ianvs/models/qwen/qwen_1_5_0_5b.gguf(on line 31) uses a relative path starting withianvs/models/....
Additionally, .gitignore (line 8) ignores models/qwen/ at the repository root. If the benchmark expects the model to be in the repository's root models/qwen/ directory, both commands should be updated to use models/qwen/ instead of /ianvs/models/qwen or ianvs/models/qwen.
Please update the wget command to use the correct relative path models/qwen/qwen_1_5_0_5b.gguf.
| wget -c -O ianvs/models/qwen/qwen_1_5_0_5b.gguf https://huggingface.co/Qwen/Qwen1.5-0.5B-Chat-GGUF/resolve/main/qwen1_5-0_5b-chat-q4_k_m.gguf | |
| wget -c -O models/qwen/qwen_1_5_0_5b.gguf https://huggingface.co/Qwen/Qwen1.5-0.5B-Chat-GGUF/resolve/main/qwen1_5-0_5b-chat-q4_k_m.gguf |
| train_data: "./examples/llm-edge-benchmark-suite/single_task_bench_with_compression/dataset/train_data/data.jsonl" | ||
| test_data: "./examples/llm-edge-benchmark-suite/single_task_bench_with_compression/dataset/test_data/data.jsonl" |
There was a problem hiding this comment.
These root-relative paths (./examples/...) assume that the ianvs command is executed from the repository root directory. However, the execution commands provided in the README.md (e.g., ianvs -f ianvs/examples/... and pip install -r ianvs/examples/...) prepend ianvs/ to the paths, implying execution from the parent directory of the repository.
To prevent path resolution failures, please ensure the documentation in README.md is updated to remove the ianvs/ prefix from the commands, so that users run them from the repository root where these relative paths will resolve correctly.
885da8a to
73787c7
Compare
…rk example Signed-off-by: Fahmid Arman <fahmid.brac@gmail.com>
73787c7 to
32b6290
Compare
|
Feedback addressed. All /assign @jaypume |
What type of PR is this?
/kind bug
What this PR does / why we need it:
The
single_task_bench_with_compressionexample referenced a dataset path from the unrelated government example, causing the benchmarking job to fail validation immediately. This PR completely restores the example for standalone execution.Changes implemented:
dataset/train_data/data.jsonlanddataset/test_data/data.jsonl) so the example runs end-to-end without requiring external downloads.testenv/testenv.yamlto point at the new dataset using root-relative paths (./examples/...)..gitignoreso this example's dataset is tracked despite the repo-widedataset/ignore rule.requirements.txtfilename that silently brokepip install.README.mdrequiring hardcoded absolute paths, fixed malformedwgetsyntax, and updated configuration steps.Which issue(s) this PR fixes:
Fixes #249