Fix StorageScheduler crash on devices without sensor in flex-model#2085
Fix StorageScheduler crash on devices without sensor in flex-model#2085saerts-gp wants to merge 1 commit intoFlexMeasures:mainfrom
Conversation
Fixes FlexMeasures#2084 Context: - StorageScheduler._prepare crashes with AttributeError when a device in the asset tree has no sensor in its flex-model (only power-capacity) - Lines 672 and 740 already guard sensor_d against None, but line 902 was missed Change: - Add 'sensor_d is not None' check before accessing event_resolution - Matches the existing pattern used elsewhere in the same method
Documentation build overview
24 files changed ·
|
|
|
||
| # Convert efficiency from sensor resolution to scheduling resolution | ||
| if sensor_d.event_resolution != timedelta(0): | ||
| if sensor_d is not None and sensor_d.event_resolution != timedelta(0): |
There was a problem hiding this comment.
I'm thinking of the case where the power sensor of the device is not mentioned in the flex-model of the API trigger message, but the device does have a storage-efficiency defined on the db flex-model.
The storage-efficiency will then become interpreted with respect to the scheduling resolution rather than the device's own power sensor resolution, and requesting the schedule in a different resolution would result in a different interpretation of the storage-efficiency over time.
This is a known shortcoming of our current implementation of the storage-efficiency (see #720), but your solution appears to me it might amplify the problem.
Four possible remedies I see going forward:
- Resolve Storage efficiency notation per any time unit. #720
- Keep requiring a sensor, but make it possible to define a power sensor in the db flex-model.
- If a storage-efficiency is defined in the db flex-model, and the corresponding power sensor is missing, throw a ValidationError.
- We document this shortcoming, for instance, in scheduling.rst under the storage-efficiency footnote, and move forward.
I'm assuming your device doesn't have a storage-efficiency, is that correct? Then 3 or 4 seem like a reasonable quick fix to me. Would you agree?
1 and/or 2 are the more long-term solutions, but they for sure require more work and whoever works on them might encounter complications along the way.
|
@saerts-gp why was this closed - is the underlying problem resolved? |
@nhoening @Flix6x Thanks for the review and apologies for closing without context. The underlying problem in FlexMeasures is not resolved — the crash at line 902 still exists on main. We worked around it on our application side by ensuring non-storage assets in the tree no longer end up with a flex_model entry that reaches the scheduler — essentially filtering them out before calling compute(). @Flix6x To answer your question: correct, the non-storage devices in our case do not have storage-efficiency defined — they only carry power-capacity. So the efficiency conversion being skipped is a no-op for our scenario. That said, I agree with your analysis that a blanket None guard could mask the problem for cases where storage-efficiency is defined but the sensor is missing. Although that issue would also be caught by testing, not necessarily crashing. Option 3 (throwing a ValidationError when storage-efficiency is present but sensor is missing) seems like the right balance to me — it protects against the silent misinterpretation you described while still not crashing on the common case of power-capacity-only entries. Again, in our case we would end up with many errors which might not be so good for our logs but we can work around that too. Happy to update the PR with the approach if you'd like, or feel free to close if you'd prefer to handle it as part of #720. |
|
I'd be glad to accept option 3 in this PR. |
3 participants
Description
AttributeError: 'NoneType' object has no attribute 'event_resolution'inStorageScheduler._prepare()when scheduling a site with non-storage devices that only havepower-capacityin their flex-model (nosensorkey)documentation/changelog.rstThe existing code at lines 672 and 740 already guards
sensor_dagainstNoneforget_attributecalls, but the efficiency conversion at line 902 was missed. This one-line fix adds the samesensor_d is not Noneguard.Look & Feel
N/A (backend-only fix)
How to test
Schedule a site asset with child assets whose
flex_modelcontains onlypower-capacity(nosensorkey). Before fix: crashes with AttributeError. After fix: completes successfully.uv run poe test -- tests/data/models/planning/Further Improvements
None.
Related Items
Closes #2084
Sign-off