Skip to content

fix: correctness & safety improvements for training - #9

Closed
alvgeppetto wants to merge 1 commit into
maderix:mainfrom
alvgeppetto:fix/safety-correctness
Closed

fix: correctness & safety improvements for training#9
alvgeppetto wants to merge 1 commit into
maderix:mainfrom
alvgeppetto:fix/safety-correctness

Conversation

@alvgeppetto

Copy link
Copy Markdown

Summary

Fixes 7 correctness & safety issues identified during M3 Ultra benchmarking and code review.

Changes

  1. ANE eval error logging (ane_runtime.h): ane_eval() now logs error details on failure via fprintf(stderr)
  2. Forward pass error checking (forward.h): ane_conv_eval changed from void to bool, returns false on eval failure
  3. Thread-safe RMSNorm (stories_cpu_ops.h): Removed global g_rms_tmp buffer, replaced with stack-local allocation
  4. Bounds checks (stories_cpu_ops.h): Added assert() guards on vocabulary indices in cross_entropy_loss, embed_lookup, embed_backward
  5. fread validation (model.h, tiny_train.m): Added FREAD_CHECK macro that validates all fread() return values during weight loading and checkpoint restore
  6. Non-destructive recompile (model.h): model_recompile_kernels now compiles into temp arrays first, swaps only on success; on failure, cleans up partial results and keeps old kernels
  7. Atomic checkpoints (tiny_train.m): save_checkpoint uses tmp+rename pattern for crash-safe writes

Testing

Both make train and make train_large compile cleanly with -Wall on macOS (Apple clang).

- Validate all fread() return values in model_load_weights (model.h)
- Check ane_eval() return values in ane_conv_eval (forward.h) and ane_eval_k (tiny_train.m)
- Log error details on ANE eval failure (ane_runtime.h)
- Thread-safe RMSNorm: replace global g_rms_tmp with local allocation (stories_cpu_ops.h)
- Bounds-check token indices in cross_entropy_loss, embed_lookup, embed_backward
- Atomic checkpoint writes via tmp+rename pattern (tiny_train.m)
- Non-destructive recompile: compile new kernels first, swap only on success (model.h)
- Validate fread() in load_checkpoint (tiny_train.m)
@alvgeppetto alvgeppetto closed this Mar 2, 2026
@alvgeppetto
alvgeppetto deleted the fix/safety-correctness branch March 2, 2026 22:44
ebowwa pushed a commit to ebowwa/ANE that referenced this pull request Aug 4, 2026
… hardening

#1 /models bypassed safety: now uses services.safe_compile (lint → validate → passes → compile)
instead of calling bridge.compile directly. The shared compile service enforces all /raw/kernels
safety rules (MIL lint, positive tensor-size validation, tensor-count limits, compiler passes,
weight-name validation, bridge-error mapping).

#2 Model leases leaked: register_model now does atomic replace (free old lease before installing
new). unregister_model calls bridge.free(kernel_id) before removing the dictionary entry.

maderix#3 Async eval bypassed admission: now calls admit_eval BEFORE dispatching the background task,
and finish_eval in _bg's finally block. max_pending backpressure applies to async work.

maderix#4 Cancellation resurrection: _bg checks sub.state == CANCELLED before transitioning to EXECUTING.
A cancelled submission is not resurrected.

maderix#5 Submission memory leak: SubmissionRegistry now has max_retained=100 + ttl_s=300. _prune()
evicts terminal submissions by TTL and count on every create().

maderix#6 Batch-eval bypassed admission: each item now goes through admit_eval/finish_eval. Endpoint
honestly named "serial batch" (NOT parallel without kernel replication).

138 tests.

Co-Authored-By: Claude <noreply@anthropic.com>
ebowwa pushed a commit to ebowwa/ANE that referenced this pull request Aug 4, 2026
maderix#9 GraphDescriptor: operation entries now include op.args (graph dependencies). Constants use
their BLOBFILE/file attr as digest instead of placeholder "const".
maderix#10 IR pipeline: rejects unparseable MIL at the tensor-IR level (rejected_at_tensor_ir) before
reaching ANE-IR lint — previously reported "validated" for meaningless input.
maderix#11 ExecutionPlan + PlanSegment: frozen=True (immutable as documented). __post_init__ uses
object.__setattr__ for the auto-generated id.
maderix#12 Communication: recv() now verifies consumer_exec_id + checksum before returning data.
maderix#13 Transforms: docstring honestly states these are VariantIntents (metadata), not transformed
graphs — quantize() labels int8, doesn't produce int8 MIL.
maderix#14 Autotuner: select_and_benchmark now passes weights to bridge.bench (previously discarded).
Evidence keyed by real mil-hash fingerprint + shape, not "auto"/"default". All-failed case
returns best_variant=None instead of picking an infinite-latency variant.

bridge.bench now accepts a weights param (for constant-weight autotuning).
138 tests.

Co-Authored-By: Claude <noreply@anthropic.com>
ebowwa pushed a commit to ebowwa/ANE that referenced this pull request Aug 4, 2026
…, capability-aware tests, byte-bounded retention, artifact manifests

maderix#5 MLX backend: load() now stores the operation spec (executable_id → spec dict). execute()
uses the stored spec, not the input's layout field. Respects per-input dtype/shape/byte_offset
(no longer assumes contiguous fp16). Produces correct output descriptors with actual output
shape (no longer reuses input descriptor).

maderix#9 Capability-aware tests: MLX tests skip when mlx unavailable or on non-Darwin-arm64
platforms. TestClient-based tests don't assert MLX availability universally.

maderix#10 Byte-bounded submissions: SubmissionRegistry now has max_result_bytes (256 MiB default).
_prune() evicts terminal submissions by total result byte size, not just count.

maderix#11 Artifact manifests: LocalArtifactBackend.put_artifact() stores each file as a tensor +
a manifest linking all files. resolve_artifact() returns the full manifest. Multi-file
artifacts (compiled ANE programs with multiple BLOBFILEs) can now be reconstructed.

149 tests.

Co-Authored-By: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant