Apply youngAge mutual exclusivity at prediction time, matching the fit - #21
Merged
Merged
Conversation
fireSense_SpreadPredict.R:282 called fireSenseUtils::spreadProbFromIntegerCovs() with
mutuallyExclusive = NULL ("already done in dataPrepPredict"), so a young pixel's fuel biomass and
non-forest land-cover columns reached the logistic unchanged instead of being zeroed alongside
youngAge = 1, as the fit requires and as fireSenseCovariatesCreate() is now fixed to actually do
upstream (PredictiveEcology/fireSenseUtils#92). Prediction now derives the same rule itself, via
the new fireSenseUtils::youngAgeExclusiveCols(), using the fuel columns it already identifies
from covMinMax_spread plus the nfLCC_*/treedWetland naming convention, so it does not depend on
covariates always arriving pre-zeroed from an upstream module.
Requires fireSenseUtils@development (>= 0.2.3.9047).
Version 1.0.0.9006.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CwcjqqK59FmTJscyi7xUqv
fireSenseUtils#92 landed as 0.2.3.9048 (development moved to 0.2.3.9047 via a concurrent PR first), so the floor here follows. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CwcjqqK59FmTJscyi7xUqv
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.
spreadProbOneELF()(fireSense_SpreadPredict.R:282) calledfireSenseUtils::spreadProbFromIntegerCovs()withmutuallyExclusive = NULL, commented "already done in dataPrepPredict". That step never actually zeroed a young non-forest pixel's land-cover value (PredictiveEcology/fireSenseUtils#92), so a young pixel's fuel biomass andnfLCC_*columns could reach the logistic unchanged instead of being cleared alongside youngAge = 1, as the fit requires.Prediction now builds the same exclusivity rule the fit uses, via the new
fireSenseUtils::youngAgeExclusiveCols(): it reuses the fuel columns this module already identifies fromcovMinMax_spread, plus the fixednfLCC_*/treedWetlandnaming convention, so the invariant holds here even if a covariate table ever arrives from somewhere other than the currentdataPrepPredictpath. RequiresfireSenseUtils@development (>= 0.2.3.9047).A new test gives a toy young pixel a fuel-range biomass and a
nfLCC_40column and checks the predicted probability matches hand-computed values with both zeroed, not the values you get if they leak through; an existing test with the same shape (a young pixel keeping fuel it should have lost) is corrected. Both fail ondevelopmentand pass here.SpaDES.core::convertToPackage()+testthat::test_local(): 44 tests / 101 expectations pass ondevelopment, 45 tests / 104 expectations pass on this branch, 0 failures either way. Version bumped to 1.0.0.9006.Merge PredictiveEcology/fireSenseUtils#92 first.
🤖 Generated with Claude Code
https://claude.ai/code/session_01CwcjqqK59FmTJscyi7xUqv