-
Notifications
You must be signed in to change notification settings - Fork 262
python-math #5: feat(math): add POLISMATH_ENGINE_MODE flag + prev-tick warm-start scaffolding (PR-A) #2618
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
jucor
wants to merge
1
commit into
spr/edge/2324207e
Choose a base branch
from
spr/edge/d61d82ad
base: spr/edge/2324207e
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
python-math #5: feat(math): add POLISMATH_ENGINE_MODE flag + prev-tick warm-start scaffolding (PR-A) #2618
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| """ | ||
| Engine-mode switch: Clojure-parity warm-start vs improved cold-recompute. | ||
|
|
||
| Python's delphi engine does a full COLD recompute on every conv-update tick. | ||
| Clojure instead THREADS warm-start state across ticks: | ||
|
|
||
| - PCA :start-vectors — the previous tick's post-normalization unit | ||
| components are fed back in as the power-iteration starting vectors | ||
| (math/src/polismath/math/conversation.clj:381-387 -> pca.clj:86-105). | ||
| - group-k-smoother — {:last-k :last-k-count :smoothed-k} state carried in | ||
| the conv, so the group count K only changes after `:group-k-buffer` (4) | ||
| consecutive ticks agree on a new K (conversation.clj:454-478). | ||
|
|
||
| `POLISMATH_ENGINE_MODE` selects between the two families: | ||
|
|
||
| - 'improved' (default): today's cold-recompute behavior, byte-for-byte. | ||
| - 'clojure-legacy' : threads the warm-start state described above. | ||
|
|
||
| The flag is resolved AT CALL TIME (never cached at import), reusing the exact | ||
| idiom of `pca._resolve_impl_flag` (pca.py:37-57): unknown values fall back to | ||
| the default with a warning so a typo in a deployment env cannot crash the math | ||
| worker. This lives in a shared spot (polismath.utils) because the mode | ||
| cross-cuts both PCA (conversation._compute_pca) and clustering | ||
| (conversation._compute_clusters). | ||
| """ | ||
|
|
||
| from typing import Sequence | ||
|
|
||
| from polismath.pca_kmeans_rep.pca import _resolve_impl_flag | ||
|
|
||
| ENGINE_MODE_ENV_VAR = 'POLISMATH_ENGINE_MODE' | ||
| ENGINE_MODE_LEGACY = 'clojure-legacy' # warm-start parity with Clojure | ||
| ENGINE_MODE_IMPROVED = 'improved' # cold recompute every tick (default) | ||
| ENGINE_MODE_DEFAULT = ENGINE_MODE_IMPROVED | ||
| ENGINE_MODE_CHOICES: Sequence[str] = (ENGINE_MODE_LEGACY, ENGINE_MODE_IMPROVED) | ||
|
|
||
|
|
||
| def resolve_engine_mode() -> str: | ||
| """ | ||
| Resolve `POLISMATH_ENGINE_MODE` from the environment, at call time. | ||
|
|
||
| Reuses `pca._resolve_impl_flag` (pca.py:37-57) so the resolution rules | ||
| (strip + lowercase, unknown -> default with a warning) are identical to | ||
| the PCA-solver switch. | ||
|
|
||
| Returns: | ||
| Either 'improved' (default) or 'clojure-legacy'. | ||
| """ | ||
| return _resolve_impl_flag( | ||
| ENGINE_MODE_ENV_VAR, ENGINE_MODE_DEFAULT, ENGINE_MODE_CHOICES) | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,114 @@ | ||
| #!/usr/bin/env python3 | ||
| """ | ||
| Tests for the POLISMATH_ENGINE_MODE switch (Clojure-parity warm-start vs | ||
| improved cold-recompute) and the cold-start invariance guard. | ||
|
|
||
| The engine mode selects between two families of behavior: | ||
| - 'improved' (default): full cold recompute every tick — today's behavior. | ||
| - 'clojure-legacy' : threads warm-start state across ticks, matching | ||
| Clojure (PCA :start-vectors, group-k-smoother). | ||
|
|
||
| On the FIRST tick (cold start) the two modes MUST coincide bit-for-bit, because | ||
| Clojure's warm-start state is empty on the first tick (no previous comps, no | ||
| smoother state). This module asserts: | ||
| 1. Flag resolution semantics (default, valid, invalid, case/whitespace, | ||
| read-at-call-time) — mirrors tests/test_powerit_pca.py::TestPcaImplFlag. | ||
| 2. Cold-start invariance: a single-shot vw pipeline run is identical under | ||
| both modes (guards commits 2 and 3 from diverging on the first tick). | ||
| """ | ||
|
|
||
| import os | ||
| import sys | ||
|
|
||
| import pytest | ||
|
|
||
| sys.path.append(os.path.abspath(os.path.dirname(__file__))) | ||
|
|
||
| from polismath.utils.engine_mode import ( | ||
| ENGINE_MODE_ENV_VAR, | ||
| ENGINE_MODE_LEGACY, | ||
| ENGINE_MODE_IMPROVED, | ||
| ENGINE_MODE_DEFAULT, | ||
| ENGINE_MODE_CHOICES, | ||
| resolve_engine_mode, | ||
| ) | ||
|
|
||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # POLISMATH_ENGINE_MODE flag resolution | ||
| # --------------------------------------------------------------------------- | ||
|
|
||
| class TestEngineModeFlag: | ||
|
|
||
| def test_default_is_improved(self, monkeypatch): | ||
| """Unset env var -> 'improved' (today's behavior is the default).""" | ||
| monkeypatch.delenv(ENGINE_MODE_ENV_VAR, raising=False) | ||
| assert resolve_engine_mode() == ENGINE_MODE_IMPROVED | ||
| assert ENGINE_MODE_DEFAULT == ENGINE_MODE_IMPROVED | ||
|
|
||
| def test_legacy_flag_selects_legacy(self, monkeypatch): | ||
| monkeypatch.setenv(ENGINE_MODE_ENV_VAR, ENGINE_MODE_LEGACY) | ||
| assert resolve_engine_mode() == ENGINE_MODE_LEGACY | ||
|
|
||
| def test_improved_flag_selects_improved(self, monkeypatch): | ||
| monkeypatch.setenv(ENGINE_MODE_ENV_VAR, ENGINE_MODE_IMPROVED) | ||
| assert resolve_engine_mode() == ENGINE_MODE_IMPROVED | ||
|
|
||
| def test_invalid_flag_value_falls_back_to_default(self, monkeypatch): | ||
| monkeypatch.setenv(ENGINE_MODE_ENV_VAR, 'not-a-mode') | ||
| assert resolve_engine_mode() == ENGINE_MODE_DEFAULT | ||
|
|
||
| def test_case_and_whitespace_insensitive(self, monkeypatch): | ||
| monkeypatch.setenv(ENGINE_MODE_ENV_VAR, ' Clojure-Legacy ') | ||
| assert resolve_engine_mode() == ENGINE_MODE_LEGACY | ||
|
|
||
| def test_flag_read_at_call_time(self, monkeypatch): | ||
| """Env var read per call, not cached at import time.""" | ||
| monkeypatch.setenv(ENGINE_MODE_ENV_VAR, ENGINE_MODE_LEGACY) | ||
| assert resolve_engine_mode() == ENGINE_MODE_LEGACY | ||
| monkeypatch.setenv(ENGINE_MODE_ENV_VAR, ENGINE_MODE_IMPROVED) | ||
| assert resolve_engine_mode() == ENGINE_MODE_IMPROVED | ||
|
|
||
| def test_choices_are_exactly_the_two_modes(self): | ||
| assert set(ENGINE_MODE_CHOICES) == {ENGINE_MODE_LEGACY, ENGINE_MODE_IMPROVED} | ||
|
|
||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # Cold-start invariance: improved vs clojure-legacy must coincide on tick 1 | ||
| # --------------------------------------------------------------------------- | ||
|
|
||
| def _strip_volatile(d): | ||
| """Remove wall-clock fields that legitimately differ between two runs. | ||
|
|
||
| `math_tick` is `25000 + (time_ms % 10000)` (conversation.py:1985) and | ||
| `last_updated` is a wall-clock timestamp — both are version counters, not | ||
| math output, so they are expected to differ between two independent runs. | ||
| """ | ||
| d = dict(d) | ||
| d.pop('last_updated', None) | ||
| d.pop('math_tick', None) | ||
| return d | ||
|
|
||
|
|
||
| class TestColdStartInvariance: | ||
| """A single-shot (first-tick) pipeline run must be byte-identical under | ||
| both engine modes. This is the hard gate protecting 'improved' mode from | ||
| any drift introduced by the legacy warm-start plumbing.""" | ||
|
|
||
| def _recompute_to_dict(self, monkeypatch, mode): | ||
| from common_utils import create_test_conversation | ||
| monkeypatch.setenv(ENGINE_MODE_ENV_VAR, mode) | ||
| conv = create_test_conversation('vw') | ||
| # Pin last_updated so the two runs share a deterministic value. | ||
| conv.last_updated = 0 | ||
| result = conv.recompute() | ||
| return _strip_volatile(result.to_dict()) | ||
|
|
||
| def test_vw_cold_run_identical_across_modes(self, monkeypatch): | ||
| improved = self._recompute_to_dict(monkeypatch, ENGINE_MODE_IMPROVED) | ||
| legacy = self._recompute_to_dict(monkeypatch, ENGINE_MODE_LEGACY) | ||
| assert improved == legacy, ( | ||
| "Cold-start (first-tick) vw run diverged between 'improved' and " | ||
| "'clojure-legacy' engine modes; warm-start state must be empty on " | ||
| "tick 1 so the two modes must coincide bit-for-bit." | ||
| ) |
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.