Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion .github/workflows/product-improvement.yml
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,20 @@ jobs:
run: |
ruff check macfleet/ tests/
python -m pytest tests/test_security tests/test_utils tests/test_cli -q
- name: Build duplicate schedule report
if: github.event_name == 'schedule' && steps.local-time.outputs.should_run == 'false'
shell: bash
run: |
set -euo pipefail
{
echo "# MacFleet product-improvement audit"
echo
echo "Generated: $(date -u +'%Y-%m-%dT%H:%M:%SZ')"
echo "Status: Duplicate schedule skipped"
echo
echo "This scheduled run was the duplicate daylight-saving fallback and did not run validation."
echo "Use the paired 3:00 AM America/New_York schedule run for the full audit."
} > product-improvement-report.md
- name: Build product-improvement report
if: github.event_name != 'schedule' || steps.local-time.outputs.should_run == 'true'
shell: bash
Expand All @@ -84,7 +98,7 @@ jobs:
echo "Use docs/automation/product-improvement-workflow.md to select one high-impact, reviewable improvement from this audit, create a human-named branch from master, test it, and open a focused PR."
} > product-improvement-report.md
- name: Upload improvement report
if: github.event_name != 'schedule' || steps.local-time.outputs.should_run == 'true'
if: github.event_name != 'schedule' || steps.local-time.outputs.should_run == 'true' || steps.local-time.outputs.should_run == 'false'
uses: actions/upload-artifact@v4
with:
name: product-improvement-report
Expand Down
10 changes: 9 additions & 1 deletion macfleet/monitoring/throughput.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import time
from collections import deque
from dataclasses import dataclass
from types import TracebackType
from typing import Optional


Expand Down Expand Up @@ -211,7 +212,14 @@ def __enter__(self) -> StepContext:
self._start = time.monotonic()
return self

def __exit__(self, *args: object) -> None:
def __exit__(
self,
exc_type: type[BaseException] | None,
_exc: BaseException | None,
_traceback: TracebackType | None,
) -> None:
if exc_type is not None:
return
end = time.monotonic()
step_time = end - self._start
compute_time = (self._compute_end - self._start) if self._compute_end > 0 else step_time
Expand Down
26 changes: 26 additions & 0 deletions macfleet/training/loop.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

from __future__ import annotations

import math
import time
from dataclasses import dataclass, field
from typing import Any, Callable, Optional
Expand All @@ -22,6 +23,7 @@

from macfleet.engines.base import Engine
from macfleet.training.data_parallel import DataParallel
from macfleet.training.guards import TrainingConfigError


@dataclass
Expand All @@ -36,6 +38,30 @@ class TrainingConfig:
on_step: Optional[Callable] = None # (step, metrics) -> None
on_epoch: Optional[Callable] = None # (epoch, metrics) -> None

def __post_init__(self) -> None:
_require_positive_int("epochs", self.epochs)
_require_non_negative_int("log_every_n_steps", self.log_every_n_steps)
_require_non_negative_int("checkpoint_every_n_steps", self.checkpoint_every_n_steps)
if (
not isinstance(self.max_grad_norm, (int, float))
or isinstance(self.max_grad_norm, bool)
or not math.isfinite(float(self.max_grad_norm))
or float(self.max_grad_norm) < 0
):
raise TrainingConfigError(
f"max_grad_norm must be a non-negative finite number, got {self.max_grad_norm!r}."
)


def _require_positive_int(name: str, value: int) -> None:
if not isinstance(value, int) or isinstance(value, bool) or value < 1:
raise TrainingConfigError(f"{name} must be a positive integer, got {value!r}.")


def _require_non_negative_int(name: str, value: int) -> None:
if not isinstance(value, int) or isinstance(value, bool) or value < 0:
raise TrainingConfigError(f"{name} must be a non-negative integer, got {value!r}.")


@dataclass
class StepResult:
Expand Down
11 changes: 11 additions & 0 deletions tests/test_monitoring/test_throughput.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,3 +187,14 @@ def test_no_compute_done_call(self):
# compute_time should equal step_time
assert tracker.avg_compute_time == pytest.approx(tracker.avg_step_time, abs=0.001)
assert tracker.avg_sync_time == 0.0

def test_failed_step_is_not_recorded(self):
tracker = ThroughputTracker()

with pytest.raises(RuntimeError, match="boom"):
with tracker.step(32):
raise RuntimeError("boom")

assert tracker.total_steps == 0
assert tracker.total_samples == 0
assert tracker.throughput_history == []
33 changes: 33 additions & 0 deletions tests/test_training/test_loop.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
"""Tests for training loop configuration validation."""

from __future__ import annotations

import pytest

from macfleet.training.guards import TrainingConfigError
from macfleet.training.loop import TrainingConfig


class TestTrainingConfig:
def test_defaults_are_valid(self):
config = TrainingConfig()

assert config.epochs == 10
assert config.log_every_n_steps == 10
assert config.checkpoint_every_n_steps == 0
assert config.max_grad_norm == 0.0

@pytest.mark.parametrize(
("kwargs", "field"),
[
({"epochs": 0}, "epochs"),
({"epochs": True}, "epochs"),
({"log_every_n_steps": -1}, "log_every_n_steps"),
({"checkpoint_every_n_steps": -1}, "checkpoint_every_n_steps"),
({"max_grad_norm": -0.1}, "max_grad_norm"),
({"max_grad_norm": float("nan")}, "max_grad_norm"),
],
)
def test_invalid_values_fail_fast(self, kwargs, field):
with pytest.raises(TrainingConfigError, match=field):
TrainingConfig(**kwargs)
12 changes: 12 additions & 0 deletions tests/test_utils/test_source_hygiene.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from pathlib import Path

ROOT = Path(__file__).resolve().parents[2]
PRODUCT_IMPROVEMENT_WORKFLOW = ROOT / ".github" / "workflows" / "product-improvement.yml"


def _package_initializers() -> list[Path]:
Expand All @@ -31,3 +32,14 @@ def test_package_initializers_do_not_contain_shell_transcripts():
offenders.append(str(path.relative_to(ROOT)))

assert offenders == []


def test_product_improvement_duplicate_schedule_uploads_report():
workflow = PRODUCT_IMPROVEMENT_WORKFLOW.read_text()

assert "Build duplicate schedule report" in workflow
assert "Duplicate schedule skipped" in workflow
assert "path: product-improvement-report.md" in workflow

upload_section = workflow.split("- name: Upload improvement report", 1)[1]
assert "steps.local-time.outputs.should_run == 'false'" in upload_section
Loading