Single owner for edge timings and variance (#72, #73)#75
Open
ff225 wants to merge 1 commit into
Open
Conversation
Two tangled problems in the same code path; the fixes share the same lines, so they land together. #72 — Edge per-layer times were smoothed twice. `track_inference_time` applied an EMA with a hard-coded alpha=0.2 to `self.inference_times` — which is the very dict `device_profiles[device_id]["edge_inference_times"]`, passed by reference — and the request handler then applied the configured EMA on top. Edge times were therefore smoothed twice while device times were smoothed once, so the two sides of the offloading cost comparison responded to change at different rates; and the `ema_alpha` setting added in #9 never reached the first application. `Edge.run_inference` already measures and returns the raw per-layer times, so the decorator's copy was redundant. It is now instrumentation only, and `DeviceStateManager.update_edge_times` is the single place that smooths, with the configured alpha. The timing there now uses perf_counter rather than time.time, since it became the authoritative measurement. #73 — `Edge.variance_detector` was a second, process-global detector fed from the same decorator. Nothing ever read it: no production code calls `should_retest_offloading()` on it, so it only burned time on the hot path and logged re-test warnings that triggered no re-test. Removed; the per-device detectors from #11 remain the only ones. Also drops the now-unwritten `inference_times` plumbing from ModelManager, `save_inference_times` (no callers), and a commented-out __init__ referencing attributes that no longer exist. `scripts/analysis/variance_analysis.py` moves to the per-device API.
This was referenced Jul 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #72. Closes #73.
Two tangled problems in the same ~15 lines, so the fixes land together.
#72 — the edge EMA was applied twice
track_inference_timeapplied an EMA with a hard-codedalpha = 0.2toself.inference_times. That dict isdevice_profiles[device_id]["edge_inference_times"], passed by reference (edge_initialization.py:140,152→model_manager.py:285). The request handler then applied the configured EMA on top of it.Two consequences:
ema_alphaconfigurable, but the first application ignored it.The fix turned out simpler than the issue suggested.
Edge.run_inferencealready measures each layer and returns the raw times — the decorator was cronometering the same call a second time. So rather than reconciling the two paths, one is removed: the decorator is now instrumentation only, andDeviceStateManager.update_edge_timesis the single place that smooths, with the configured alpha.#73 — a second variance detector nobody read
Edge.variance_detectorwas a process-global detector fed from the same decorator. No production code callsshould_retest_offloading()on it — every such call is in tests. It burned time on the hot path and logged re-test warnings that triggered no re-test. Removed; the per-device detectors from #74 are now the only ones.Fallout cleanup
With the decorator no longer writing,
ModelManager.inference_timeshad no writers left, so the plumbing goes: theinference_times/variance_detectorparams, theedge_inference_timesargument threaded throughEdge.run_inferenceand_get_model_manager,save_inference_times(no callers), and a commented-out__init__referencing attributes that no longer exist.Two judgement calls worth a look
time.time()→perf_counter()inrun_inference. Now that this is the authoritative measurement of edge cost, a monotonic clock is the right tool — but it is a change nobody asked for.scripts/analysis/variance_analysis.pymoved to the per-device API. Extract per-device state out of RequestHandler (#11) #74 broke it (it readRequestHandler.variance_detector) and I missed it there. Worth knowing: that script reads the running server's in-memory state from a separate process, so it always found an empty detector. This makes it coherent, it does not make it useful.Testing
244 passed, 1 skipped (fast suite + HTTP end-to-end). New
tests/unit/test_edge_inference_time_ownership.pypins the single-EMA behaviour and the configured alpha. ruff clean on touched files (the 8 findings invariance_analysis.pyare pre-existing — confirmed againstmain).Edge.run_inferenceis not exercised by the fast suite — its real tests are markedmodel_artifact. The regression tests here cover signatures and the EMA maths, but the edge inference chain itself has not been run end-to-end. Worth a run with the model artifacts before merging.