fix: keep shared-tank sources active when operating_hours == 0#975
fix: keep shared-tank sources active when operating_hours == 0#975Thimpey wants to merge 1 commit into
Conversation
Reviewer's GuideAdjusts deferrable-load activation logic so shared-tank thermal sources are never deactivated by operating-hours or window masks, and adds regression tests to cover both infeasibility paths for shared thermal tanks. File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
perform_optimization,shared_tank_membershipis used withk in shared_tank_membership; if this structure can ever be large, consider turning the membership keys into asetonce so the repeatedinchecks stay cheap and explicit. - The new warning in the window-mask reset path will fire every horizon where a shared-tank load has an out-of-horizon window; consider either throttling it (e.g. debug-level or once-per-load) or including a clearer hint that this is an expected/benign behavior when using temperature-driven sources.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `perform_optimization`, `shared_tank_membership` is used with `k in shared_tank_membership`; if this structure can ever be large, consider turning the membership keys into a `set` once so the repeated `in` checks stay cheap and explicit.
- The new warning in the window-mask reset path will fire every horizon where a shared-tank load has an out-of-horizon window; consider either throttling it (e.g. debug-level or once-per-load) or including a clearer hint that this is an expected/benign behavior when using temperature-driven sources.
## Individual Comments
### Comment 1
<location path="src/emhass/optimization.py" line_range="3644-3647" />
<code_context>
+ # min_temperatures unreachable (infeasible, then the relaxed
+ # fallback fails too and nothing is published).
+ if k in shared_tank_membership and window_outside_horizon:
+ if k < len(self.param_window_masks):
+ self.param_window_masks[k].value = np.ones(n)
+ self.logger.warning(
+ "Deferrable load %d is a shared-tank source with a configured "
</code_context>
<issue_to_address>
**suggestion:** Derive the window mask shape from the existing parameter instead of hardcoding np.ones(n).
To better future-proof this, construct the mask from `param_window_masks[k].value`’s shape (e.g. `np.ones_like(self.param_window_masks[k].value)` or `np.ones(self.param_window_masks[k].value.shape)`) rather than assuming a 1D length-`n` vector, so you avoid potential shape mismatches if the mask dimensionality changes.
```suggestion
if k in shared_tank_membership and window_outside_horizon:
if k < len(self.param_window_masks):
self.param_window_masks[k].value = np.ones_like(self.param_window_masks[k].value)
self.logger.warning(
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| if k in shared_tank_membership and window_outside_horizon: | ||
| if k < len(self.param_window_masks): | ||
| self.param_window_masks[k].value = np.ones(n) | ||
| self.logger.warning( |
There was a problem hiding this comment.
suggestion: Derive the window mask shape from the existing parameter instead of hardcoding np.ones(n).
To better future-proof this, construct the mask from param_window_masks[k].value’s shape (e.g. np.ones_like(self.param_window_masks[k].value) or np.ones(self.param_window_masks[k].value.shape)) rather than assuming a 1D length-n vector, so you avoid potential shape mismatches if the mask dimensionality changes.
| if k in shared_tank_membership and window_outside_horizon: | |
| if k < len(self.param_window_masks): | |
| self.param_window_masks[k].value = np.ones(n) | |
| self.logger.warning( | |
| if k in shared_tank_membership and window_outside_horizon: | |
| if k < len(self.param_window_masks): | |
| self.param_window_masks[k].value = np.ones_like(self.param_window_masks[k].value) | |
| self.logger.warning( |
Shared-tank members are temperature-driven. Master already exempts them from the running_lb pinning and the energy constraint, but the param_load_active activation loop still used `is_thermal = k in self.param_thermal` only. A shared-tank source is not in param_thermal, so with operating_hours == 0 (the natural setting for a temperature-driven load) it was deactivated: p_deferrable[k] forced to 0 for the whole horizon. With every member off the tank cannot hold its min_temperatures band, the problem goes infeasible, the relaxed fallback is infeasible too, and nothing is published. Same fix for the window-mask path: a member whose configured window falls entirely outside the horizon had its mask zeroed (same 0-W pin); reset it to all-ones and warn, since temperature constraints drive the load. Hoists shared_tank_membership into perform_optimization scope and adds it to the is_thermal exemption, matching the existing energy-constraint exemption. Two regression tests reproduce both infeasibility paths and fail on master without this change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1aa2412 to
952d5c1
Compare
Fixes #974.
Shared-tank sources are temperature-driven, but the
param_load_activeactivation loop inperform_optimizationonly treatedk in self.param_thermalas thermal. A shared-tank source is not inparam_thermal, so withoperating_hours == 0(the natural setting for a temperature-driven load) it was deactivated and pinned to 0 W for the whole horizon — the tank then cannot hold itsmin_temperaturesband, the problem goes infeasible, the relaxed fallback is infeasible too, and nothing is published.Master already exempts shared-tank members from the running-lb pinning and the energy constraint, so this just adds the same exemption to the third place that missed it. Same class as #887.
Changes
shared_tank_membershipintoperform_optimizationscope and add it to theis_thermalexemption in the activation loop.The repro and before/after output are in #974. Full test suite passes locally (the one unrelated
test_open_meteo_legacy_pvlibfailure is pre-existing on master) andruff check .is clean.Happy to adjust the approach if you'd prefer the exemption handled differently.
Co-authored with Claude Code (Anthropic).
Summary by Sourcery
Ensure temperature-driven shared-tank deferrable loads remain active and feasible under zero operating hours and out-of-horizon windows.
Bug Fixes:
Tests: