Repository navigation
Conversation
…e pair fx_market prices each FX print from the pair's other prints (leave-one-out) and re-extracted every peer record, twice, for each print; the swap_trades forex priceability gate does that for every print of the day, so a forex swap_trades call was quadratic in a pair's prints: over an hour of CPU on a day of ~43k FX prints, holding the event loop the whole time. option_vol likewise re-extracted and re-implied every vanilla option of the pair for each option print. fx_market now prepares each pair once per call. The extraction's only cross-record input is its median anchor, and leaving out one print leaves one of a few possible anchors, so each anchor's oriented, carry-checked observations are built once and a print only re-takes the medians its own observation touches. Markets carry a key (pair, anchor, left-out prints, min_trades); equal keys mean equal markets, and option_vol prices a family once per key. Results are unchanged. A new test checks every print's spot and forward against extract_fx_observations + ndf_forward_curve over its peers; on a real 2026-10-08 DTCC file, 889 sampled prints (506 forwards, 383 options) gave field-for-field the same value_fx_trade output, and all 57,536 records valued in 5.1 s. Tests that stubbed the extraction helpers now stub _peer_curve, the new seam, with unchanged assertions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
|
|
Could a maintainer approve the workflow runs for this PR? They're waiting for approval because it comes from a fork ( |
dlee-bq
left a comment
dlee-bq
left a comment
There was a problem hiding this comment.
Hi @artcashin, thanks for the PR. I've left some comments in the code regarding code duplication through reimplementations. Please have a look and refactor accordingly.
Thanks
| return _cip_market(peers, rates_records, trade, curve_date, min_trades, memo) | ||
| return _cip_market(curve["cip_near"], rates_records, trade, curve_date, min_trades, memo, key) | ||
|
|
||
| spot, forward = ndf_forward_curve( |
There was a problem hiding this comment.
The shared utility function was removed and replaced with an inline copy, making maintenence more difficult. For consistency, use the shared utility. While we are at it, can you please align the Vol Surface and Valuation to use the same NDF curve - with package legs?
| return at(mid) if n % 2 else (at(mid - 1) + at(mid)) / 2 | ||
|
|
||
|
|
||
| def _pair_view(fx_records: Iterable[dict], trade: dict, curve_date: dateType, memo: dict) -> dict: |
There was a problem hiding this comment.
This, along with _anchored, re-implement the anchor, orient, and carry-check steps of extract_fx_observations. Let's remove re-implementations.
|
|
||
| def _median_without(ordered: list[float], removed: list[float]) -> float | None: |
There was a problem hiding this comment.
This reimplements statistics.median, which is already part of extract_fx_observations. Let's remove the redundancies.
| def _pair_view(fx_records: Iterable[dict], trade: dict, curve_date: dateType, memo: dict) -> dict: | ||
| """A pair's curve observations, prepared once per day for leave-one-out pricing. | ||
|
|
||
| fx_market prices each print from its peers (the pair's other prints). | ||
| Re-extracting every peer per print is quadratic in a busy pair's day. The | ||
| extraction's only cross-record input is its median anchor, and leaving out | ||
| one print leaves one of a few possible anchors, so each anchor's oriented, | ||
| carry-checked observations are built once and each print only adjusts the | ||
| medians its own observation touches. | ||
| """ | ||
| from openbb_cftc.utils.fx import _fx_observation, resolve_pair | ||
|
|
||
| key = f"fx-view:{trade['pair']}" |
There was a problem hiding this comment.
fx-view:{pair} is keyed without curve_date, so the first call’s date fixes the tenors for the memo’s life.
Address review: drop the leave-one-out reimplementation (_pair_view, _anchored, _median_without and the inline forward interpolator). fx_market now calls extract_fx_observations and ndf_forward_curve once per pair and curve date over all of the pair's prints, package legs included, so valuation and the vol surface use the same NDF curve. The curve, market and option-pricing memo keys include the curve date. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SEna2ixXYhEGEAQdJM2D3G
This branch has not been deployed
3 participants
Problem
fx_marketprices each FX print from the pair's other prints, leaving the print itself out. It re-extracts every peer record, twice, for each print. Theswap_tradesforex priceability gate calls it for every print of the day, so oneGET /cftc/swap_trades?asset_class=forexis quadratic in a pair's prints. On a day of about 43,000 FX prints (USDKRW alone has 12,231) it ran for over an hour at 100% CPU.option_volhas the same shape: it re-extracts and re-implies every vanilla option of the pair for each option print.Change
fx_marketnow prepares each pair once per call, with the same results.min_trades), and equal keys mean equal markets. Option prints never enter the forward curve, so all option prints of a pair share one market, andoption_volprices each family once per key.Results unchanged
test_fx_market_matches_the_leave_one_out_curve: on 61 synthetic KRW prints, including a shared identifier and prints outside the carry bound, every print's spot and forward at six tenors equalsextract_fx_observations+ndf_forward_curveover its peers, exactly.test_option_vol_prices_a_family_once_per_market.value_fx_tradeoutput as before.Speed
On the same file:
swap_tradesendpointTests
Eight existing tests stubbed
extract_fx_observationsandndf_forward_curve, whichfx_marketno longer calls. They now stub_peer_curve, the new seam, through the shared_patch_markethelper, with their assertions unchanged. The own-print exclusion and memo tests now use real records.tests/test_fx_valuation.pypasses 93/93, and the CFTC suite passes 1,227/1,227.🤖 Generated with Claude Code