Fix hillSlope1 at 1: not identifiable with covariate coefficients - #93
Merged
Merged
Conversation
hillSlope1 is not identifiable together with the covariate coefficients: with the linear predictor x = covariates %*% beta, hillSlope1 enters logistic3p()/logistic3pUpper() only as hillSlope1 * x, so scaling every coefficient by k and dividing hillSlope1 by k leaves every prediction unchanged. .objfunSpreadFit() now calls the new internal fixHillSlope1() on every par it receives, reinserting hillSlope1 = 1 as the 2nd logistic parameter, so callers no longer include it in DEoptim's lower/upper. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CwcjqqK59FmTJscyi7xUqv
eliotmcintire
force-pushed
the
fix/hillSlope1-fixed-at-1
branch
from
September 28, 2026 17:40
91237ff to
2310a3e
Compare
eliotmcintire
added a commit
to PredictiveEcology/fireSense_spreadFit
that referenced
this pull request
Sep 28, 2026
fireSenseUtils' development moved to 0.2.3.9047 (unrelated PR) while PredictiveEcology/fireSenseUtils#93 was open, so its fix rebased onto 0.2.3.9048; match the floor here. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CwcjqqK59FmTJscyi7xUqv
…ix/hillSlope1-fixed-at-1 # Conflicts: # NEWS.md
fixHillSlope1() has no man page (not exported), so [fixHillSlope1()] could not resolve; use plain code formatting instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CwcjqqK59FmTJscyi7xUqv
eliotmcintire
changed the base branch from
development
to
fix/youngage-exclusivity
September 28, 2026 17:47
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## development #93 +/- ##
===============================================
+ Coverage 44.35% 44.38% +0.02%
===============================================
Files 40 40
Lines 3803 3805 +2
===============================================
+ Hits 1687 1689 +2
Misses 2116 2116 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This was referenced Sep 28, 2026
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.
The spread link's slope, hillSlope1, was a DEoptim-fitted parameter (fireSenseUtils/R/objFunSpread.R, via estimateSpreadParams() in fireSense_SpreadFit) with bounds [0.2, 2], but it is not identifiable together with the covariate coefficients. With the linear predictor x = covariates %*% beta, hillSlope1 enters logistic3p()/logistic3pUpper() only as hillSlope1 * x, so scaling every coefficient by k and dividing hillSlope1 by k leaves every prediction unchanged, and DEoptim let coefficients drift along that ridge.
.objfunSpreadFit() (R/objFunSpread.R) now calls a new internal fixHillSlope1() on every par it receives, right after the yearSpreadSD element is stripped, reinserting hillSlope1 = 1 as the second logistic parameter before the parameters are split and evaluated. Callers (fireSense_SpreadFit) stop including hillSlope1 in DEoptim's lower/upper.
Verified with two new tests (test-fixHillSlope1.R): fixHillSlope1() inserts hillSlope1 = 1 at the right position for both named and unnamed input, and logisticAll() on the reconstructed vector gives the same prediction as calling it with hillSlope1 fixed at 1 by hand. test-yearSpreadSD.R is updated to expect the inserted hillSlope1 in the vector .objfunSpreadFit() hands to objFunInner(). Full suite, stacked on PR #92 (fix/youngage-exclusivity), scratch lib: 299 test blocks / 802 passed, 0 failed.
This branch is stacked on #92 (fix/youngage-exclusivity), so this PR's base is that branch, not development; it will retarget automatically once #92 merges. Version 0.2.3.9049. This pairs with PredictiveEcology/fireSense_spreadFit#47, similarly stacked, which needs this version.
🤖 Generated with Claude Code
https://claude.ai/code/session_01CwcjqqK59FmTJscyi7xUqv