Repository navigation
One ELF rescales with its ledger row's covMinMax_spread - #29
Merged
Merged
Conversation
…inMax_spread The single-ELF path used sim$covMinMax_spread, which fireSense_spreadFit rebuilds in every run from that run's covariate tables; in a hindcast those follow a formula rebuilt for another year, so a fitted term could have no range and silently stay on its raw scale. ledgerCovMinMax() takes the row's ranges (the fit's), like ledgerCovCentre(); a row without them (an older ledger) still uses sim$covMinMax_spread. A fitted covariate with no range stops. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CwcjqqK59FmTJscyi7xUqv
eliotmcintire
added
bug
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
1 participant
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.
A single fitted fire region now rescales its prediction covariates with the ranges stored with its fit, as several regions already did. Before, it used ranges rebuilt by the fitting module in the current run, which in a hindcast could lack a fitted term and leave it unscaled without any message.
Affects: Predictions are unchanged wherever the rebuilt ranges equalled the fit's (every current NRV run); a hindcast now rescales exactly as the fit did, and stops if a fitted covariate has no range.
Technical details
Defect:
fireSense_spreadPredict.R:154passedsim$covMinMax_spreadfor one ELF, while the multi-ELF path (:175) usedsa$covMinMax_spread[[i]].fireSense_spreadFitrunsspreadFitPrep()in every run (itsspreadFitPrepareevent, :426, also when the ledger has the row) and rebuildssim$covMinMax_spreadfrom the run'sfireSense_spreadFormula; a hindcast's formula can differ from the fit's terms (fireSense_dataPrepFit#69).fireSenseUtils::spreadProbFromIntegerCovs()rescales only the columns ofcovMinMax, so a fitted term it lacked stayed on its raw x1000 scale.Fix:
ledgerCovMinMax(sa, i, default), besideledgerCovCentre(), returns the row'scovMinMax_spread, elsedefault(sim$covMinMax_spread, for a row written before ranges were stored).spreadProbOneELF()stops when a fitted covariate has no range. Reused:ledgerCovCentre()pattern,paramSetForRep(),spreadProbOneELF().Tests: new
test-ledgerCovMinMax.R(row ranges used, fallback for a row without them, stop on a missing range; the first fails on development: the prediction ignores the row's ranges). Module suite 58 tests, 0 failures (convertToPackage+test_local).🤖 Generated with Claude Code
https://claude.ai/code/session_01CwcjqqK59FmTJscyi7xUqv