Skip to content

Fix model-level edfct derivatives for absolute ED type#188

Merged
hreinwald merged 4 commits intodevfrom
copilot/fix-standard-errors-absolute-ed
Apr 13, 2026
Merged

Fix model-level edfct derivatives for absolute ED type#188
hreinwald merged 4 commits intodevfrom
copilot/fix-standard-errors-absolute-ed

Conversation

Copy link
Copy Markdown

Copilot AI commented Apr 13, 2026

When type = "absolute", each model's edfct function sets ∂ED/∂c and ∂ED/∂d to 0. This is wrong — the absolute-to-relative conversion (p = 100·(d−respl)/(d−c)) makes p a function of c and d, so the chain rule requires non-zero partials. The ED.drc caller already patches this with a numerical override, but the model-level functions themselves remain incorrect for direct callers.

Changes

  • braincousens.R, fplogistic.R, llogistic.R, llogistic2.R, lnormal.R, weibull1.R, weibull2.R: When type == "absolute", compute ∂ED/∂c and ∂ED/∂d via central differences on a closure that re-evaluates the full ED computation with perturbed c/d, capturing the chain-rule contribution through EDhelper/absToRel.

  • weibull2.R: Special handling because it delegates to weibull1$edfct with type="relative". The .edval wrapper replicates both weibull2's reversal (b > 0) and weibull1's EDhelper swap (b < 0) to match the actual computation path.

  • logistic.R: Previously did not accept reference/type at all — absolute ED estimates were silently wrong. Added reference/type parameters with an inline absolute-to-relative conversion instead of EDhelper, because the logistic model has the opposite b-sign convention from log-logistic (b < 0 = increasing, not decreasing). EDhelper's p-swap for b < 0 would incorrectly swap ED values for the logistic model, so only the raw conversion p = 100·(d−respl)/(d−c) is applied for absolute type, with p = respl used directly for relative type.

Pattern applied in each file

# After computing EDder with analytical derivatives (which have 0 for c,d)...
if (identical(type, "absolute")) {
    .edval <- function(pv) {
        p0 <- EDhelper(pv, respl, reference, type)
        # ... model-specific ED formula using p0 ...
    }
    .eps <- .Machine$double.eps
    for (.i in c(2, 3)) {
        .h <- if (abs(parmVec[.i]) > sqrt(.eps)) abs(parmVec[.i]) * .eps^(1/3) else .eps^(1/3)
        .pvUp <- replace(parmVec, .i, parmVec[.i] + .h)
        .pvDn <- replace(parmVec, .i, parmVec[.i] - .h)
        EDder[.i] <- (.edval(.pvUp) - .edval(.pvDn)) / (2 * .h)
    }
}

No changes to type = "relative" behavior. Verified model-level SE matches ED() output for LL.4, W1.4, W2.4, LN.4. All 152 ED tests, 83 ED_robust tests pass.

Copilot AI and others added 2 commits April 13, 2026 08:42
…evel edfct functions

When type='absolute', the edfct functions in model files set derivatives for
c (lower limit) and d (upper limit) to 0. This is incorrect because the
absolute-to-relative conversion makes p depend on c and d, requiring chain-rule
corrections. Use central differences to compute correct c and d derivatives
for absolute type in all affected model files.

Fixes: braincousens.R, fplogistic.R, llogistic.R, llogistic2.R, lnormal.R,
weibull1.R, weibull2.R, logistic.R

Agent-Logs-Url: https://github.com/hreinwald/drc/sessions/7ea3ad01-6a69-4d8a-8c99-5b419f07b01f

Co-authored-by: hreinwald <115988583+hreinwald@users.noreply.github.com>
…legation path

Agent-Logs-Url: https://github.com/hreinwald/drc/sessions/7ea3ad01-6a69-4d8a-8c99-5b419f07b01f

Co-authored-by: hreinwald <115988583+hreinwald@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix incorrect standard errors for absolute ED calculations Fix model-level edfct derivatives for absolute ED type Apr 13, 2026
Copilot AI requested a review from hreinwald April 13, 2026 08:55
@hreinwald
Copy link
Copy Markdown
Owner

There seems to me an issue with the unit tests. @copilot please review the errors from the R-CMD-check which all failed for the unit test (testthat) section. Identify the issue and fix it.

…d incorrect p-swap

The logistic model has opposite b-sign convention from log-logistic: b < 0 means
increasing (not decreasing). EDhelper swaps p for b < 0, which is correct for
log-logistic but wrong for logistic. Replace with inline absolute-to-relative
conversion that skips the p-swap.

Agent-Logs-Url: https://github.com/hreinwald/drc/sessions/801bd853-cb45-4ae2-8e75-c28dae361ad5

Co-authored-by: hreinwald <115988583+hreinwald@users.noreply.github.com>
Copy link
Copy Markdown
Author

Copilot AI commented Apr 13, 2026

There seems to me an issue with the unit tests. @copilot please review the errors from the R-CMD-check which all failed for the unit test (testthat) section. Identify the issue and fix...

The R-CMD-check failures were all in test-logistic.R (lines 213 and 218) — the ED10/ED90 values were swapped.

Root cause: I incorrectly added EDhelper() to the logistic model's edfct. The logistic model has the opposite b-sign convention from log-logistic: b < 0 = increasing curve, not decreasing. EDhelper swaps p → 100−p for b < 0, which is correct for log-logistic but wrong for logistic, causing ED10 and ED90 values to swap.

Fix (aef7e9f): Replaced EDhelper() with an inline absolute-to-relative conversion that only does p = 100·(d−respl)/(d−c) for type="absolute" and p = respl for type="relative" — no p-swap.

@hreinwald hreinwald marked this pull request as ready for review April 13, 2026 11:04
@hreinwald hreinwald merged commit 4e4a37a into dev Apr 13, 2026
6 checks passed
@hreinwald hreinwald deleted the copilot/fix-standard-errors-absolute-ed branch April 13, 2026 11:05
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.

Incorrect Standard Errors for Absolute ED Calculations

2 participants