fix(fuelModels): correct isDynamic for static standard fuel models#66
Open
jimmynotjim wants to merge 1 commit into
Open
fix(fuelModels): correct isDynamic for static standard fuel models#66jimmynotjim wants to merge 1 commit into
jimmynotjim wants to merge 1 commit into
Conversation
populateFuelModels() hardcoded isDynamic=true for the entire 101-204 Scott & Burgan block, including the 23 models with no live herbaceous load (SH2-8, TU2/4/5, TL1-9, SB1-4). Updated those to false and added a test asserting isDynamic == (fuelLoadLiveHerbaceous > 0) across the standard set.
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.
Disclosure: I typically work in JS/TS rather than C++ so I'm not personally familiar with the workings of this library. I utilized Claude Code to discover and assist in writing, running, and confirming these fixes. I know a lot of OSS repos are receiving a lot of AI Slop these days, but I tried to be discriminating in my use here to just this issue and to confirm the bug and the fix via tests.
Purpose
FuelModels::populateFuelModels()insrc/behave/fuelModels.cpppassedisDynamic = truetosetFuelModelRecord(...)for every standard Scott & Burgan (2005) model (101–204), regardless of whether the model is actually dynamic. This PR flipsisDynamictofalsefor the 23 static models that have no transferable live herbaceous load, and adds a unit test that locks in the correct behavior.The 23 corrected records: SH2–SH8 (142–148), TU2/TU4/TU5 (162, 164, 165), TL1–TL9 (181–189), SB1–SB4 (201–204). The genuinely dynamic models (GR1–9, GS1–4, SH1, SH9, TU1, TU3) were already correct and are unchanged.
Impact
The values are utilized as metadata only.
isDynamiconly does work when there's live herbaceous load to transfer; since all 23 records havefuelLoadLiveHerbaceous = 0, the load-transfer step was already a no-op for them. Fire-behavior outputs (rate of spread, flame length, etc.) are unaffected. The defect was purely in the reportedgetIsDynamic()attribute, which is why it went unnoticed. It only happened to surface downstream where the flag is read as metadata.Related Issues
Closes #65
Submission Checklist
make compile)make test)Testing
Adds
testFuelModelDynamicFlags()tosrc/testBehave/testBehave.cpp, asserting the invariant for the 40 standard models:The extended SCAL/V-/M-/F- models are deliberately excluded — their static/dynamic designation doesn't follow this rule.
188 passed / 23 failed.211 passed / 0 failed. No existing test regressed.