Skip to content

Commit ffdbc28

Browse files
committed
Refactor: unify PTO-ISA resolve + auto-clone; add shared --log-level helper
PTO-ISA unification ------------------- Before this change there were two parallel implementations of "ensure PTO-ISA is available": simpler_setup/pto_isa.py::ensure_pto_isa_root() - Only looked up (env var -> default path), raised OSError if missing. - Called by scene_test.py, so `pytest` and standalone `python test_*.py` hit OSError on a fresh checkout, with an error message that literally told the user to go run ci.py or run_example.py first. A real onboarding papercut. simpler_setup/code_runner.py::_ensure_pto_isa_root() - Full clone + commit pin + lock + fetch-latest logic (~280 lines). - Called by ci.py and run_example.py (via CodeRunner). Unify both behind a single API in simpler_setup/pto_isa.py. All four user entry points (pytest, standalone test_*.py, ci.py, run_example.py) now auto-clone on first use. Move default clone target examples/scripts/_deps/pto-isa -> PROJECT_ROOT/build/pto-isa - `build/` is the repo's canonical artifact directory (already gitignored, already hosts `build/lib/` and `build/cache/`). - Per-repo/worktree/venv isolation via PROJECT_ROOT means concurrent worktrees each get their own clone and don't race on commit pins. - One-time re-clone for existing dev machines; acceptable. New signature: ensure_pto_isa_root( commit: Optional[str] = None, clone_protocol: str = "ssh", update_if_exists: bool = False, # fetch origin/HEAD when True + no commit verbose: bool = False, ) -> str - scene_test calls with defaults -> auto-clone on first run, never touch an existing clone (no network on every pytest run). - ci.py / CodeRunner pass update_if_exists=True + verbose=True to preserve "always ensure latest" behaviour; commit pin still wins when provided. code_runner.py loses ~280 lines of PTO-ISA code and the fcntl import; ci.py updates its two import sites to reach pto_isa directly (ensure_pto_isa_root, checkout_pto_isa_commit, get_pto_isa_clone_path). The "Cloning pto-isa to X ..." heads-up is emitted at WARNING level so it surfaces even in pytest / standalone paths that have no basicConfig — otherwise users would see a 30-60s silent hang on first run. Shared --log-level helper ------------------------- Add simpler_setup/log_config.py exposing LOG_LEVEL_CHOICES, DEFAULT_LOG_LEVEL ("info"), and configure_logging(log_level). All three CLI entry points now share one spelling of the flag, all default to INFO: ci.py --log-level error|warn|info|debug examples/scripts/run_example.py --log-level ... (plus -v / --silent shortcuts) python test_*.py (run_module) --log-level ... The helper also propagates PTO_LOG_LEVEL env so ci.py's runtime-isolation subprocesses inherit the level. pytest is untouched — it has its own --log-cli-level / pyproject log_cli_level and doesn't need a layered flag. Verification ------------ - pytest tests/ut/py: 121 passed, 5 skipped (unchanged from main) - tools/verify_packaging.sh: all 5 install modes × 4 entry points green - All 3 CLI entry points expose --log-level choices with default info
1 parent 554bf89 commit ffdbc28

6 files changed

Lines changed: 327 additions & 374 deletions

File tree

ci.py

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,7 @@
6060
from threading import Lock, Thread
6161
from typing import Any, Callable, Protocol, cast
6262

63-
PROJECT_ROOT = Path(__file__).resolve().parent
64-
65-
from simpler.task_interface import ( # noqa: E402 # type: ignore[import-not-found]
63+
from simpler.task_interface import ( # type: ignore[import-not-found]
6664
ChipCallable, # pyright: ignore[reportAttributeAccessIssue]
6765
ChipCallConfig, # pyright: ignore[reportAttributeAccessIssue]
6866
ChipStorageTaskArgs, # pyright: ignore[reportAttributeAccessIssue]
@@ -72,6 +70,10 @@
7270
scalar_to_uint64,
7371
)
7472

73+
from simpler_setup.log_config import DEFAULT_LOG_LEVEL, LOG_LEVEL_CHOICES, configure_logging
74+
75+
PROJECT_ROOT = Path(__file__).resolve().parent
76+
7577
logger = logging.getLogger("ci")
7678

7779
# ---------------------------------------------------------------------------
@@ -264,16 +266,17 @@ def discover_tasks(platform: str, runtime_filter: str | None = None) -> list[Tas
264266

265267

266268
def ensure_pto_isa(commit: str | None, clone_protocol: str) -> str:
267-
from simpler_setup.code_runner import _ensure_pto_isa_root # noqa: PLC0415
268-
269-
root = _ensure_pto_isa_root(verbose=True, commit=commit, clone_protocol=clone_protocol)
270-
if root is None:
271-
raise OSError(
272-
"PTO_ISA_ROOT could not be resolved.\n"
273-
"Set it manually or let auto-clone run:\n"
274-
" export PTO_ISA_ROOT=$(pwd)/examples/scripts/_deps/pto-isa"
275-
)
276-
return root
269+
from simpler_setup.pto_isa import ensure_pto_isa_root # noqa: PLC0415
270+
271+
# update_if_exists=True: when no commit is pinned, fetch latest origin/HEAD
272+
# so CI runs reproducibly track main rather than whatever local checkout
273+
# happens to be on disk.
274+
return ensure_pto_isa_root(
275+
commit=commit,
276+
clone_protocol=clone_protocol,
277+
update_if_exists=True,
278+
verbose=True,
279+
)
277280

278281

279282
# ---------------------------------------------------------------------------
@@ -952,11 +955,11 @@ def print_summary(results: list[TaskResult]) -> int:
952955

953956
def reset_pto_isa(commit: str, clone_protocol: str) -> str:
954957
"""Checkout PTO-ISA at the pinned commit (or re-clone if needed)."""
955-
from simpler_setup.code_runner import _checkout_pto_isa_commit, _get_pto_isa_clone_path # noqa: PLC0415
958+
from simpler_setup.pto_isa import checkout_pto_isa_commit, get_pto_isa_clone_path # noqa: PLC0415
956959

957-
clone_path = _get_pto_isa_clone_path()
960+
clone_path = get_pto_isa_clone_path()
958961
if clone_path.exists():
959-
_checkout_pto_isa_commit(clone_path, commit, verbose=True)
962+
checkout_pto_isa_commit(clone_path, commit, verbose=True)
960963
return str(clone_path.resolve())
961964
return ensure_pto_isa(commit, clone_protocol)
962965

@@ -1177,6 +1180,9 @@ def parse_args() -> argparse.Namespace:
11771180
parser.add_argument("-t", "--timeout", type=int, default=600)
11781181
parser.add_argument("--clone-protocol", choices=["ssh", "https"], default="ssh")
11791182
parser.add_argument("--all", dest="run_all_cases", action="store_true", help="Run all cases, not just DEFAULT_CASE")
1183+
parser.add_argument(
1184+
"--log-level", choices=LOG_LEVEL_CHOICES, default=DEFAULT_LOG_LEVEL, help="Root logger level (default: info)"
1185+
)
11801186
parser.add_argument("--device-worker", action="store_true", help=argparse.SUPPRESS)
11811187
parser.add_argument("--result-json", default=None, help=argparse.SUPPRESS)
11821188
parser.add_argument("--task-list-json", default=None, help=argparse.SUPPRESS)
@@ -1276,9 +1282,8 @@ def _run_single_platform(platform: str, args: argparse.Namespace) -> list[TaskRe
12761282

12771283

12781284
def main() -> int:
1279-
logging.basicConfig(level=logging.INFO, format="[%(levelname)s] %(message)s", force=True)
1280-
12811285
args = parse_args()
1286+
configure_logging(args.log_level)
12821287
args.devices = parse_device_range(args.device_range)
12831288

12841289
valid_platforms = _discover_valid_platforms()

examples/scripts/run_example.py

Lines changed: 11 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,9 @@
4040
import time
4141
from pathlib import Path
4242

43+
from simpler_setup.code_runner import create_code_runner
44+
from simpler_setup.log_config import DEFAULT_LOG_LEVEL, LOG_LEVEL_CHOICES, configure_logging
45+
4346
project_root = Path(__file__).parent.parent.parent
4447

4548
logger = logging.getLogger(__name__)
@@ -131,19 +134,20 @@ def compute_golden(tensors: dict, params: dict) -> None:
131134
"-v",
132135
"--verbose",
133136
action="store_true",
134-
help="Enable verbose output (equivalent to --log-level debug)",
137+
help="Shortcut for --log-level debug",
135138
)
136139

137140
parser.add_argument(
138141
"--silent",
139142
action="store_true",
140-
help="Silent mode - only show errors (equivalent to --log-level error)",
143+
help="Shortcut for --log-level error",
141144
)
142145

143146
parser.add_argument(
144147
"--log-level",
145-
choices=["error", "warn", "info", "debug"],
146-
help="Set log level explicitly (overrides --verbose and --silent)",
148+
choices=LOG_LEVEL_CHOICES,
149+
default=None,
150+
help=f"Root logger level (default: {DEFAULT_LOG_LEVEL}). Overrides --verbose / --silent.",
147151
)
148152

149153
parser.add_argument(
@@ -206,31 +210,16 @@ def compute_golden(tensors: dict, params: dict) -> None:
206210
if args.all and args.case:
207211
parser.error("--all and --case are mutually exclusive")
208212

209-
# Determine log level from arguments
210-
log_level_str = None
213+
# --log-level takes priority; -v / --silent are shortcuts
211214
if args.log_level:
212215
log_level_str = args.log_level
213216
elif args.verbose:
214217
log_level_str = "debug"
215218
elif args.silent:
216219
log_level_str = "error"
217220
else:
218-
log_level_str = "info"
219-
220-
# Setup logging before any other operations
221-
level_map = {
222-
"error": logging.ERROR,
223-
"warn": logging.WARNING,
224-
"info": logging.INFO,
225-
"debug": logging.DEBUG,
226-
}
227-
log_level = level_map.get(log_level_str.lower(), logging.INFO)
228-
229-
# Configure Python logging
230-
logging.basicConfig(level=log_level, format="[%(levelname)s] %(message)s", force=True)
231-
232-
# Set environment variable for C++ side
233-
os.environ["PTO_LOG_LEVEL"] = log_level_str
221+
log_level_str = DEFAULT_LOG_LEVEL
222+
configure_logging(log_level_str)
234223

235224
# Validate paths
236225
kernels_path = Path(args.kernels)
@@ -249,10 +238,7 @@ def compute_golden(tensors: dict, params: dict) -> None:
249238
logger.error(f"kernel_config.py not found in {kernels_path}")
250239
return 1
251240

252-
# Import and run
253241
try:
254-
from simpler_setup.code_runner import create_code_runner # noqa: PLC0415
255-
256242
runner = create_code_runner(
257243
kernels_dir=str(args.kernels),
258244
golden_path=str(args.golden),

0 commit comments

Comments
 (0)