fix(features): deterministic, symbol-aware IV estimate (#9)#31
Open
bradsmithmba wants to merge 1 commit into
Open
fix(features): deterministic, symbol-aware IV estimate (#9)#31bradsmithmba wants to merge 1 commit into
bradsmithmba wants to merge 1 commit into
Conversation
ImpliedVolatilityEstimator.estimate_iv() multiplied a per-category base
volatility by random.uniform(0.8, 1.2) seeded on int(time.time()) % 100,
so the same symbol returned different IVs on consecutive calls and no Greek
or POP derived from it was reproducible. get_iv_for_position() also called
estimate_iv("SPY") unconditionally, applying a low-vol ETF base (0.15) to
every position regardless of the real underlying.
- Remove the wall-clock-seeded random factor; return the deterministic
per-category base (ETF 0.15 / large-cap tech 0.25 / general 0.30).
- Add an optional `symbol` argument to get_iv_for_position so the real
underlying flows through; when it is unknown, fall back to the general
single-stock base rather than assuming SPY.
Verified: estimate_iv('AAPL') == estimate_iv('AAPL') == 0.25; position IV
now differs by symbol. Adds a determinism test.
Closes cloudtrainerwork#9
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author
|
Merge-ordering note: the determinism test added here lives in |
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.
Summary
ImpliedVolatilityEstimatorhad two compounding defects insrc/features/greeks.py:Non-deterministic.
estimate_iv()multiplied a per-category base volatility by a random factor seeded on wall-clock time:The same symbol returned different IVs on consecutive calls, so no Greek or probability-of-profit derived from it was reproducible across a run.
SPY hardcoded.
get_iv_for_position()calledestimate_iv("SPY")unconditionally, applying a low-vol ETF base (0.15) to every position regardless of the real underlying — systematically underestimating IV for individual names.Closes #9.
Fix
estimate_iv()returns the deterministic per-category base (ETF 0.15 / large-cap tech 0.25 / general 0.30).symbolargument toget_iv_for_position(position, symbol=None). When the underlying is known it flows through; when unknown, fall back to the general single-stock base (0.30) instead of assuming SPY. (Positioncarries no symbol field today, which is why the method had nothing to pass — the new parameter lets callers supply it.)Verification
Exercised directly (the test file can't be collected on
masteryet — see below):A determinism test (
test_iv_estimation_is_deterministic) is added totests/test_greeks.py.CI note
tests/test_greeks.pyimportssrc.features.greeks, which triggerssrc/features/__init__.py→ the data layer missing until #1 (PR #19) merges, so the file can't be collected onmasteryet. The fix was verified by loading the module directly (it depends only onposition_models, numpy, and scipy). The new test runs in CI once #1 lands.🤖 Generated with Claude Code