Repository navigation
fix(predheat): allow temperature forecasts without heating-energy history - #5867
liambresnahan wants to merge 4 commits into
Conversation
springfall2008
left a comment
There was a problem hiding this comment.
Automated comment from the triage bot.
Reviewed at a5a2e92. The fix is correct: minute_data_age's only consumer is get_historical() (apps/predbat/predheat.py:276), the new history_ages list always has the three temperature entries so min() cannot go empty, and the truthiness of self.heating_energy is the right emptiness test — minute_data_entity() returns ({}, 0) only when the entity is unconfigured or its history yields no points, in which case the age carries no span information the replay could use. This resolves the second bug recorded from the #5153 review in tools/debug-journal.md (heating_energy unset pins minute_data_age to 0, so get_historical() always returns its 20.0 default), and the code now matches docs/predheat.md:103, which already says heating_energy can be commented out. By reading the new test against main's line: min() includes the 0 from the energy stub, minute_data_age becomes 0, use_days becomes 0, total_weight stays 0 and the 20.0 default is returned — both assertions fire, so the test is genuinely red/green against the fix. heat_energy_today/today_cost/run_simulation all degrade safely on the empty dict (0 so far today, early return, 0 cost). Registry entry added in unit_test.py ✓.
1 low finding posted inline. Notes, not findings: (a) the energy-present-but-short case is preserved as the body promises — an energy entity only hours old still floors the replay to 0 (age.days), but that self-heals as recorder history accumulates, unlike the unconfigured case this PR fixes; per-series ages inside get_historical() would be the deeper generalisation if that variant starts reporting, and the prune-blip path into the same 20.0 fallback is already recorded as #5456. (b) With the fix, schedule replay now actually runs in energy-less setups, so the pre-existing span-alignment behaviour of get_from_history() (missing minutes read as 0) becomes reachable there too; minute_data_age already caps the replay to each run's dense span, and the residual exposure is the same #5456 prune-blip entrance, unchanged by this PR.
|
Docstring issue resolved |
Summary
Predheat currently includes the age of
heating_energyhistory when calculatingminute_data_age, even when no heating-energy entity is configured or it has no data.minute_data_entity()returns({}, 0)in that case, so the minimum age is always zero. As a result,get_historical()ignores otherwise valid indoor/outdoor temperature and thermostat setpoint history and the forecast falls back to the 20°C default target.This change makes heating-energy history optional for that age calculation:
Includes a focused regression test covering missing energy, shorter energy history, and missing target history, registered in the existing Predbat test runner.
Testing
predheat_optional_energy_historyinapps/predbat/tests/test_predheat.py.main: onlyapps/predbat/predheat.py,apps/predbat/tests/test_predheat.pyandapps/predbat/unit_test.pychanged.coverage/run_all --test predheat_optional_energy_history, red/green reproduction, andcoverage/run_pre_commit. The current environment cannot clone the repository or execute its test harness. Please treat CI and the regression test as pending, not passed.Notes
PredHeat.get_historical()in the existing Predheat forecast simulation; callers were reviewed from source.