feat: add detect-compute.py — hardware probing for ML feasibility #24

Merged

magnus merged 2 commits from feat/detect-compute-script into main 2026-05-23 17:16:24 -04:00

Conversation 2 Commits 2 Files changed 4 +633

magnus commented 2026-05-23 17:05:54 -04:00

Owner

Copy link

Phase 1 of #22

Adds scripts/detect-compute.py — a standalone CLI that probes the environment and outputs structured JSON recommendations for:

• Model size tier (based on VRAM)
• Feasible techniques (full FT, LoRA, QLoRA, distillation, sklearn)
• Batch size guidance
• Quantization availability
• Distillation feasibility

Test results: 12/12 passing (7 local + 5 Docker)

Key design decisions:

• Graceful degradation on all probes (no GPU, no torch, no psutil)
• Subprocess probes avoid import-side-effect contamination
• --minimal flag for quick agent injection
• Docker test image verifies no-GPU, no-torch scenario

To test:
bash scripts/test_detect_compute.sh

**Phase 1 of #22** Adds `scripts/detect-compute.py` — a standalone CLI that probes the environment and outputs structured JSON recommendations for: - Model size tier (based on VRAM) - Feasible techniques (full FT, LoRA, QLoRA, distillation, sklearn) - Batch size guidance - Quantization availability - Distillation feasibility **Test results:** 12/12 passing (7 local + 5 Docker) **Key design decisions:** - Graceful degradation on all probes (no GPU, no torch, no psutil) - Subprocess probes avoid import-side-effect contamination - `--minimal` flag for quick agent injection - Docker test image verifies no-GPU, no-torch scenario **To test:** `bash scripts/test_detect_compute.sh`

magnus added 2 commits 2026-05-23 17:05:55 -04:00

chore: resolve merge conflict after pulling origin main (data-scientist merged, epub added locally) 5ed56721c6

feat: add detect-compute.py — hardware probing for ML feasibility ... fe8127f353

Standalone CLI that probes GPU (nvidia-smi), CUDA version, PyTorch,
scikit-learn, JAX, Optuna, RAM, and disk space — then generates
structured recommendations for model size, batch size, quantization,
and distillation feasibility.

Ships with 12-test suite (7 local + 5 Docker) covering graceful
degradation, all output flags, and a containerized no-GPU scenario.

Part of #22

magnus referenced this pull request 2026-05-23 17:13:06 -04:00

Data-Scientist Skill v2: Experimental Campaign Pipeline + Infrastructure Awareness + Subagent Supervision #22

magnus merged commit edb1e7d9bf into main 2026-05-23 17:16:24 -04:00

magnus referenced this pull request from a commit 2026-05-23 17:16:25 -04:00

Merge pull request 'feat: add detect-compute.py — hardware probing for ML feasibility' (#24) from feat/detect-compute-script into main

jasper reviewed 2026-05-23 17:17:00 -04:00

jasper left a comment

Copy link

Code Review — detect-compute.py

Overall: Solid implementation. The graceful degradation pattern is well-executed, the subprocess probe approach correctly avoids import-side-effect contamination, and the Docker test coverage is a nice touch. A few issues to address:

1. Default output mode mismatch (detect-compute.py:390-403)

The docstring says default is "Pretty-printed system overview" but the else branch (line 401) does json.dumps(info, indent=2, default=str) — the same as --json. The default should run _format_verbose(info) or a trimmed human-readable version, not dump JSON. Either fix the output or fix the docstring.

2. Scope creep — epub unrelated to detect-compute (AGENTS.md, README.md)

AGENTS.md and README.md are modified to add epub skill references. These are unrelated to the PR title "feat: add detect-compute.py" from issue #22. Should be a separate PR.

3. Redundant import shutil inside _get_disk_free_mb (detect-compute.py:~225)

shutil is already imported at module level (line 14). The try/except re-import inside the function is unnecessary — shutil.disk_usage is always available from the module-level import.

4. Redundant .strip() on already-stripped stdout (detect-compute.py:~143)

_run() already strips stdout before returning. The stdout.strip() in the _nvidia_smi parsing loop is redundant.

5. Dockerfile uses slim image (test_detect_compute.sh)

Minor: python:3.12-slim may not have pip pre-installed in all versions. Consider python:3.12-slim-bookworm which has it.

Recommendation: Address issues 1-2 before merge. Issues 3-5 are minor.

Jasper (automated review)

## Code Review — detect-compute.py **Overall:** Solid implementation. The graceful degradation pattern is well-executed, the subprocess probe approach correctly avoids import-side-effect contamination, and the Docker test coverage is a nice touch. A few issues to address: ### 1. Default output mode mismatch (detect-compute.py:390-403) The docstring says default is "Pretty-printed system overview" but the else branch (line 401) does `json.dumps(info, indent=2, default=str)` — the same as `--json`. The default should run `_format_verbose(info)` or a trimmed human-readable version, not dump JSON. Either fix the output or fix the docstring. ### 2. Scope creep — epub unrelated to detect-compute (AGENTS.md, README.md) AGENTS.md and README.md are modified to add epub skill references. These are unrelated to the PR title "feat: add detect-compute.py" from issue #22. Should be a separate PR. ### 3. Redundant import shutil inside `_get_disk_free_mb` (detect-compute.py:~225) `shutil` is already imported at module level (line 14). The try/except re-import inside the function is unnecessary — `shutil.disk_usage` is always available from the module-level import. ### 4. Redundant .strip() on already-stripped stdout (detect-compute.py:~143) `_run()` already strips stdout before returning. The `stdout.strip()` in the `_nvidia_smi` parsing loop is redundant. ### 5. Dockerfile uses slim image (test_detect_compute.sh) Minor: `python:3.12-slim` may not have `pip` pre-installed in all versions. Consider `python:3.12-slim-bookworm` which has it. **Recommendation:** Address issues 1-2 before merge. Issues 3-5 are minor. _Jasper (automated review)_

jasper reviewed 2026-05-23 17:28:27 -04:00

jasper left a comment

Copy link

Code Review Summary — PR #24

What it does

Adds data-scientist/scripts/detect-compute.py — a standalone hardware/software probe for ML feasibility. Outputs structured JSON with model size tier, feasible techniques, batch size guidance, quantization options, and distillation feasibility. 12/12 tests passing.

Strengths

• Graceful degradation on every probe path — no unhandled crashes if nvidia-smi, torch, psutil, or /proc/meminfo are absent
• Subprocess isolation for ML library probes avoids import-side-effect contamination
• Docker test coverage validates the no-GPU, no-torch scenario end-to-end
• Clean exit code conventions (0 = clean, 1 = degraded environment)
• Good flag ergonomics: --json, --minimal, --verbose, --list-gpus, --help

Issues Found (5 inline comments posted)

#	Severity	File	Line	Issue
1	Minor	detect-compute.py	138	Redundant import shutil — already imported at module level
2	Minor	detect-compute.py	247	notes field concatenation can produce leading space " PyTorch..." when no threshold note was set
3	Low	detect-compute.py	93	sys.executable isolation — may miss torch in venvs if script runs from system Python
4	Low	detect-compute.py	36	--list-gpus vs FLAGS key list_gpus inconsistency; --list_gpus silently ignored
5	Low	AGENTS.md	58	Unrelated epub skill entries in a detect-compute PR — scope creep

Additional Observations

• --minimal --json returns recommendations JSON nested under info["recommendations"]; --minimal without --json does the same thing — consistent behavior ✓
• _get_disk_free_mb() returns KB from df fallback (column 3) not MB, because the result is free // (1024 * 1024) while df -P already outputs KB. The fallback returns 0 instead of a correct value. (Divide by 1024, not 1024*1024.)
• The VRAM thresholds (24GB, 16GB, 8GB, 4GB) are reasonable defaults that cover most modern consumer/workstation GPUs.

Verdict

Clean, well-structured addition. Issues are minor — none block merge. The disk-free calculation in the df fallback path is the only substantive bug.

Reviewer: Jasper (automated review)

## Code Review Summary — PR #24 ### What it does Adds `data-scientist/scripts/detect-compute.py` — a standalone hardware/software probe for ML feasibility. Outputs structured JSON with model size tier, feasible techniques, batch size guidance, quantization options, and distillation feasibility. 12/12 tests passing. ### Strengths - **Graceful degradation** on every probe path — no unhandled crashes if nvidia-smi, torch, psutil, or /proc/meminfo are absent - **Subprocess isolation** for ML library probes avoids import-side-effect contamination - **Docker test coverage** validates the no-GPU, no-torch scenario end-to-end - Clean exit code conventions (0 = clean, 1 = degraded environment) - Good flag ergonomics: `--json`, `--minimal`, `--verbose`, `--list-gpus`, `--help` ### Issues Found (5 inline comments posted) | # | Severity | File | Line | Issue | |---|----------|------|------|-------| | 1 | Minor | detect-compute.py | 138 | Redundant `import shutil` — already imported at module level | | 2 | Minor | detect-compute.py | 247 | `notes` field concatenation can produce leading space `" PyTorch..."` when no threshold note was set | | 3 | Low | detect-compute.py | 93 | `sys.executable` isolation — may miss torch in venvs if script runs from system Python | | 4 | Low | detect-compute.py | 36 | `--list-gpus` vs FLAGS key `list_gpus` inconsistency; `--list_gpus` silently ignored | | 5 | Low | AGENTS.md | 58 | Unrelated epub skill entries in a detect-compute PR — scope creep | ### Additional Observations - **`--minimal --json`** returns recommendations JSON nested under `info["recommendations"]`; `--minimal` without `--json` does the same thing — consistent behavior ✓ - **`_get_disk_free_mb()`** returns KB from `df` fallback (column 3) not MB, because the result is `free // (1024 * 1024)` while `df -P` already outputs KB. The fallback returns 0 instead of a correct value. (Divide by 1024, not 1024*1024.) - The VRAM thresholds (24GB, 16GB, 8GB, 4GB) are reasonable defaults that cover most modern consumer/workstation GPUs. ### Verdict Clean, well-structured addition. Issues are minor — none block merge. The disk-free calculation in the `df` fallback path is the only substantive bug. **Reviewer:** Jasper (automated review)

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

community-feedback

Originates from community review

  

enhancement

New feature or request

  

skill-upgrade

Skill enhancement or refactor

No labels

community-feedback

enhancement

skill-upgrade

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

magnus/agent-skills!24

No description provided.