Skip to content

fix(pk): terminal t½ window selection; delete two dead code paths#100

Merged
jam-sudo merged 3 commits into
mainfrom
chore/minors-deadcode-nca-thalf
Jul 5, 2026
Merged

fix(pk): terminal t½ window selection; delete two dead code paths#100
jam-sudo merged 3 commits into
mainfrom
chore/minors-deadcode-nca-thalf

Conversation

@jam-sudo

@jam-sudo jam-sudo commented Jul 4, 2026

Copy link
Copy Markdown
Owner

Three verified minor review items, all headline-neutral (Cmax/AUC unchanged). Each verified before acting — two other "minor" claims (solubility mol/L-vs-mg/mL, an RBP asymmetry severity) were checked in the same pass and handled differently (see below).

1. pk/nca.py — terminal t½ fit on a selected terminal window (real bug)

terminal_half_life regressed the log-linear slope over all post-Cmax points, blending the steep distribution (α) phase into λz → t½ biased short for multi-compartment disposition. Now selects the terminal window by the adjusted-R² "best fit" rule (Phoenix/WinNonlin): among last-k-point windows (k≥3), maximize adjusted R², preferring more points on a tie. A mono-exponential curve still uses the whole tail (behavior unchanged).

t½ is a secondary endpoint (does not feed the headline Cmax); its one numeric consumer is MIPD steady-state dose sizing. Added a bi-exponential regression test the old all-points fit fails (returns ≈ln2/α instead of ln2/β).

2. predict/drugbank.py — delete dead get_ugt_enzymes

Zero callers; superseded by the JSON substrate registry (non_cyp_substrates). Also removed its now-dead plumbing: _ugt_substrates store, the UGT populate block, and the unused _UGT_NORMALIZATION map.

3. predict/chemistry.py — delete dead XGBoost pKa loader

_load_pka_models / _predict_pka_from_model / _PKA_MODEL_DIR were never wired into compute_profile (which uses DrugBank pKa + the SMARTS _estimate_pka_type fallback). The model was tested and reverted (worsened AAFE).

Left intact (verified, out of scope)

  • graph merge_overlay stub + sc_overlay.yaml/tumor_overlay.yaml — dead today, but an intentional, documented Phase-3 extension seam (matched stub + fixture pair). Not cruft; kept.
  • solubility units — claim falsified: mg/mL end-to-end; consumed only via a coarse radius bucket, no molar/dose-number term exists. No change.
  • RBP reset asymmetry — real (low severity) but on the SMILES-only path the headline benchmark uses, so a fix changes Cmax and needs a canonical CI regen; tracked separately, not folded in here.

Verification

test_endpoints (12, incl. the new bi-exponential test) + test_drugbank + test_drugbank_integration + test_features_chemistry (61) pass; ruff clean; no residual references to the deleted symbols.

…ad code paths

Three verified minor review items, all headline-neutral (Cmax/AUC unchanged).

1. pk/nca.py — terminal_half_life fit the log-linear slope over ALL post-Cmax
   points, blending the steep distribution (α) phase into λz and biasing t½
   short for multi-compartment disposition. Now selects the terminal window by
   the adjusted-R² "best fit" rule (Phoenix/WinNonlin): among last-k-point
   windows (k≥3), maximize adjusted R², preferring more points on a tie. A
   mono-exponential curve still uses the whole tail (unchanged). t½ is a
   secondary endpoint (does not feed the headline Cmax); its one numeric consumer
   is MIPD steady-state dose sizing. Added a bi-exponential regression test that
   the old all-points fit fails (returns ln2/α instead of ln2/β).

2. predict/drugbank.py — delete dead get_ugt_enzymes (+ its _ugt_substrates
   store, the UGT populate block, and the now-unused _UGT_NORMALIZATION map).
   Zero callers; UGT fm is sourced from the JSON substrate registry
   (non_cyp_substrates), not DrugBank.

3. predict/chemistry.py — delete dead _load_pka_models / _predict_pka_from_model
   / _PKA_MODEL_DIR (the XGBoost pKa loader). Never wired into compute_profile
   (which uses DrugBank pKa + the SMARTS _estimate_pka_type fallback); the model
   was tested and reverted (worsened AAFE).

Left intact (verified but out of scope): the graph merge_overlay stub +
sc/tumor overlay YAMLs (intentional Phase-3 extension seam), and RBP reset (real
but on the headline benchmark path → needs a canonical regen, tracked separately).

Tests: test_endpoints (12) + test_drugbank + test_drugbank_integration +
test_features_chemistry (61) pass; ruff clean.
@jam-sudo
jam-sudo enabled auto-merge (squash) July 4, 2026 22:41

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d531cb7b61

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/sisyphus/pk/nca.py
y_hat = slope * t_w + intercept
ss_res = float(np.sum((y_w - y_hat) ** 2))
ss_tot = float(np.sum((y_w - y_w.mean()) ** 2))
r2 = 1.0 if ss_tot <= 0.0 else 1.0 - ss_res / ss_tot

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Skip flat terminal windows

When the last three post-Cmax samples are an identical positive floor (for example an assay/numerical LLOQ tail like [10, 5, 2, 1e-9, 1e-9, 1e-9]), ss_tot <= 0.0 makes that flat window look like a perfect adjusted-R² fit; np.polyfit can still return a tiny negative roundoff slope, so the new selector picks the plateau and returns an enormous t½ (~6e14 h) instead of using the preceding declining tail or returning None. Flat/no-variance windows should be rejected (or require a meaningful negative slope) rather than scored as perfect terminal decline.

Useful? React with 👍 / 👎.

@jam-sudo
jam-sudo merged commit d37880d into main Jul 5, 2026
2 checks passed
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.

1 participant