Skip to content

perf: limit trend directional movement scan#22

Open
ShauryaMallampati wants to merge 2 commits into
Dragoon4002:mainfrom
ShauryaMallampati:perf/market-trend-tail-dm
Open

perf: limit trend directional movement scan#22
ShauryaMallampati wants to merge 2 commits into
Dragoon4002:mainfrom
ShauryaMallampati:perf/market-trend-tail-dm

Conversation

@ShauryaMallampati

Copy link
Copy Markdown

Refs #2.

Summary

  • optimize MarketAnalyzer trend metrics by computing only the final 14 directional-movement values used by the simplified ADX calculation
  • avoid building full plus/minus DM arrays for the entire medium window on each feature computation
  • add regression coverage comparing the optimized path against the previous full-array calculation

Benchmark

Local synthetic OHLCV inputs, best of 5 repeats:

  • 2,016 medium candles / 5,000 calls: old 2.147981s vs new 0.040188s, 53.45x faster
  • 2,016 medium candles / 1,000 calls: old 0.430773s vs new 0.008103s, 53.16x faster

Validation

  • .venv/bin/python -m pytest shared/tests/test_market_analysis.py -q
  • .venv/bin/python -m py_compile app/market/market_analysis.py shared/tests/test_market_analysis.py
  • git diff --check

Copilot AI review requested due to automatic review settings July 5, 2026 07:03
@vercel

vercel Bot commented Jul 5, 2026

Copy link
Copy Markdown

@ShauryaMallampati is attempting to deploy a commit to the dragoon4002's projects Team on Vercel.

A member of the Team first needs to authorize it.

@ShauryaMallampati

Copy link
Copy Markdown
Author

Follow-up on checks: the visible Vercel status is blocked on repo-owner/team authorization, and the GitHub Actions workflows are waiting for maintainer approval to run on this forked PR.

Local validation for this patch:

  • .venv/bin/python -m pytest shared/tests/test_market_analysis.py -q -> 2 passed
  • .venv/bin/python -m py_compile app/market/market_analysis.py shared/tests/test_market_analysis.py
  • git diff --check

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR optimizes MarketAnalyzer trend-metric computation by avoiding full-window directional-movement array materialization and instead aggregating only the final values needed for the simplified ADX calculation, with a new regression test intended to validate equivalence vs the prior approach.

Changes:

  • Optimize simplified ADX directional-movement computation by summing only the last 14 deltas instead of building full plus_dm/minus_dm arrays.
  • Add a reference implementation in tests and compare optimized vs full-array behavior for both long and short windows.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
app/market/market_analysis.py Limits directional-movement scanning to the last 14 deltas and computes ADX from rolling sums instead of full arrays.
shared/tests/test_market_analysis.py Adds regression tests that compare the optimized trend-metrics path against a reference full-array calculation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread app/market/market_analysis.py Outdated
Comment on lines +190 to +194
# Directional movement. The simplified ADX below only uses the
# final 14 values, so skip older bars instead of materializing
# full-window arrays for every feature computation.
plus_dm_sum = 0
minus_dm_sum = 0
Comment on lines +73 to +83
def test_compute_trend_metrics_matches_reference_full_dm_arrays():
candles = _candles(2016)
recent = candles[-288:]
medium = candles[-2016:]
long = candles

assert MarketAnalyzer._compute_trend_metrics(recent, medium, long) == _reference_trend_metrics(
recent,
medium,
long,
)
@ShauryaMallampati

Copy link
Copy Markdown
Author

Addressed the Copilot review comments in 90c15bd:

  • clarified that the simplified ADX path uses up to the final 14 directional-movement values
  • moved the regression test into tests/ so it is included by the repo CI command (pytest tests/ ...)

Validation:

  • .venv/bin/python -m pytest tests/test_market_analysis.py -q -> 2 passed
  • .venv/bin/python -m pytest tests/ -q -> 2 passed
  • .venv/bin/python -m py_compile app/market/market_analysis.py tests/test_market_analysis.py
  • git diff --check

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.

2 participants