-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Switch to 15m slots for tariffs and planner #21162
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
I'd say the value is always |
|
Wie besprochen schaue ich mir die Visualisierung des Ladeplans an. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `core/site_optimizer.go:391` </location>
<code_context>
- result := make([]float64, 0, 24)
+// prorateFirstSlot strips away any slots before "now" and prorates the first slot based on remaining time in current slot.
+// The profile contains 48 15min slots (00:00-23:45) that repeat for multiple days.
+func prorateFirstSlot(profile []float64, firstSlotDuration time.Duration) []float64 {
+ firstSlot := int(time.Now().Truncate(tariff.SlotDuration).Sub(now.BeginningOfDay()) / tariff.SlotDuration)
</code_context>
<issue_to_address>
**issue:** prorateFirstSlot assumes profile is long enough; consider bounds check.
Slicing profile[firstSlot:] without checking length may cause a panic if profile is too short. Please add a bounds check to prevent this.
</issue_to_address>
### Comment 2
<location> `tariff/slots_test.go:124` </location>
<code_context>
+ assert.Equal(t, expected, out)
+}
+
+func TestDropOldRates(t *testing.T) {
+ now := time.Now().Truncate(SlotDuration)
+
</code_context>
<issue_to_address>
**suggestion (testing):** Test for dropping old rates is a good addition.
Please also add a test for rates that start before 'now' and end after, to confirm partial overlaps are handled correctly.
Suggested implementation:
```golang
func TestDropOldRates(t *testing.T) {
now := time.Now().Truncate(SlotDuration)
}
func TestPartialOverlapRates(t *testing.T) {
now := time.Now().Truncate(SlotDuration)
// Create a rate that starts before 'now' and ends after 'now'
rate := api.Rate{
Start: now.Add(-SlotDuration),
End: now.Add(SlotDuration),
Value: 5.0,
}
rates := []api.Rate{rate}
// Call the function under test (assuming DropOldRates or similar)
out := DropOldRates(rates, now)
// We expect the returned rate to start at 'now' and end at rate.End
expected := []api.Rate{
{
Start: now,
End: rate.End,
Value: rate.Value,
},
}
assert.Equal(t, expected, out)
}
```
- If the function under test is not named `DropOldRates`, replace it with the correct function name.
- Ensure that the test setup matches the actual logic for handling partial overlaps in your codebase.
- If your code expects more than one slot or rate in the output, adjust the expected slice accordingly.
</issue_to_address>
### Comment 3
<location> `tariff/slots_test.go:148-150` </location>
<code_context>
+//
+// For solar tariffs we expect power at time of interval start (see https://github.com/evcc-io/evcc/issues/23184 for changing this).
+// When converting to 15min slots, solar interpolation needs to take care of this
+func TestSolarAndCo2Interpolation(t *testing.T) {
+ now := time.Now().Truncate(SlotDuration)
+
</code_context>
<issue_to_address>
**suggestion (testing):** Solar interpolation test is well-designed.
Consider adding a test case for api.TariffTypeCo2 to ensure CO2 interpolation behaves as expected, since the comment references CO2 but only solar is tested.
```suggestion
func TestSolarAndCo2Interpolation(t *testing.T) {
now := time.Now().Truncate(SlotDuration)
// Solar interpolation test
solarTariff := testTariff{
typ: api.TariffTypeSolar,
rates: []api.Rate{{Start: now, Value: 100}, {Start: now.Add(SlotDuration), Value: 200}},
}
solarSlots := InterpolateSlots(solarTariff, now, now.Add(2*SlotDuration), SlotDuration)
require.Len(t, solarSlots, 2)
assert.Equal(t, 100.0, solarSlots[0].Value)
assert.Equal(t, 200.0, solarSlots[1].Value)
// CO2 interpolation test
co2Tariff := testTariff{
typ: api.TariffTypeCo2,
rates: []api.Rate{{Start: now, Value: 50}, {Start: now.Add(SlotDuration), Value: 75}},
}
co2Slots := InterpolateSlots(co2Tariff, now, now.Add(2*SlotDuration), SlotDuration)
require.Len(t, co2Slots, 2)
assert.Equal(t, 50.0, co2Slots[0].Value)
assert.Equal(t, 75.0, co2Slots[1].Value)
```
</issue_to_address>
### Comment 4
<location> `core/site_optimizer.go:362` </location>
<code_context>
// homeProfile returns the home base load in Wh
-func (site *Site) homeProfile(minLen int) []float64 {
+func (site *Site) homeProfile(minLen int, firstSlotDuration time.Duration) ([]float64, error) {
// kWh over last 30 days
profile, err := metrics.Profile(now.BeginningOfDay().AddDate(0, 0, -30))
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring the homeProfile logic into a single loop that fetches, prorates, and emits slots in one pass.
```suggestion
The `homeProfile` path can be collapsed into a single loop—no giant intermediate slice, no threading `firstSlotDuration`, no separate `combineSlots`/`prorateFirstSlot`. You just need to:
1. fetch your 15 min‐slot pattern once,
2. compute the current slot index and fraction remaining,
3. emit exactly `minLen` slots in one pass (prorating slot 0 inline),
4. convert to Wh at the end.
For example:
```go
func (site *Site) homeProfile(minLen int) ([]float64, error) {
// 1) fetch raw 15 min slots over last 30 days
profile, err := metrics.Profile(now.BeginningOfDay().AddDate(0, 0, -30))
if err != nil {
return nil, err
}
const daySlots = 24 * 4
// 2) compute current slot index & proration factor
now := time.Now()
slotIdx := now.Hour()*4 + now.Minute()/15
// how much of the current 15 min is left
rem := tariff.SlotDuration - time.Duration(now.Minute()%15)*time.Minute
frac := float64(rem) / float64(tariff.SlotDuration)
// 3) single-pass fill
res := make([]float64, minLen)
for i := 0; i < minLen; i++ {
v := profile[(slotIdx+i)%daySlots]
if i == 0 {
v *= frac
}
// 4) convert kWh to Wh
res[i] = v * 1e3
}
return res, nil
}
```
Then drop `combineSlots`, `prorateFirstSlot`, and the extra `firstSlotDuration` parameter entirely. This preserves all existing behavior with far less slicing and looping.
</issue_to_address>
### Comment 5
<location> `tariff/slots.go:19` </location>
<code_context>
+// Slot length must be multiple of SlotDuration.
+// For price tariffs, the value is constant over all sub-slots.
+// For solar/co2, linear interpolation is used between slot boundaries.
+func (t *SlotWrapper) Rates() (api.Rates, error) {
+ rates, err := t.Tariff.Rates()
+ if err != nil {
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring the Rates method by extracting slot splitting and interpolation logic into helper functions for improved clarity.
Here’s one possible refactoring that peels out the two main responsibilities—splitting into fixed buckets and computing/interpolating a value—into small helpers. The `Rates` method then becomes a single loop that’s easy to follow.
```go
// splitTimes returns all start times for 15m slots covering [start,end).
func splitTimes(start, end time.Time, slot time.Duration) []time.Time {
var times []time.Time
// align to slot boundary
cur := start.Truncate(slot)
for cur.Before(end) {
times = append(times, cur)
cur = cur.Add(slot)
}
return times
}
// interpolate returns a linear interpolation at t between a and b.
func interpolate(a, b api.Rate, t time.Time) float64 {
total := b.Start.Sub(a.Start).Seconds()
if total <= 0 {
return a.Value
}
frac := t.Sub(a.Start).Seconds() / total
return a.Value + frac*(b.Value-a.Value)
}
func (t *SlotWrapper) Rates() (api.Rates, error) {
in, err := t.Tariff.Rates()
if err != nil {
return nil, err
}
var out api.Rates
now := time.Now().Truncate(SlotDuration)
for i, r := range in {
if !r.End.After(now) {
continue
}
for _, start := range splitTimes(r.Start, r.End, SlotDuration) {
val := r.Value
if (t.Type() == api.TariffTypeSolar || t.Type() == api.TariffTypeCo2) && i+1 < len(in) {
val = interpolate(r, in[i+1], start)
}
out = append(out, api.Rate{Start: start, End: start.Add(SlotDuration), Value: val})
}
}
return out, nil
}
```
Steps:
1. Extract `splitTimes(start, end, slot)` to get all slot‐start times.
2. Extract `interpolate(a, b, t)` for linear interpolation.
3. In `Rates()`, replace the double‐loop + switch with a single loop that calls these helpers.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
We need to document the 15min slot change for custom tariffs: https://docs.evcc.io/en/docs/tariffs#plugin |





fixes #23962
At October 1st the EPEX SPOT Day Ahead market switched to 15min price slots. In this PR we're adjusting our price handling to reflect this.
⏳ API/UI always uses 15min intervals for tariff and forecast
🎯 planner respects 15min slots (actually: it always did!)
🌎 support for tariffs with 1h or 15min slot data
💰 1h price tariff input data will be recalculated in 15min slots (4 equals)
🌱 1h CO2 tariff input data will be recalculated in 15min slots (4 equals)*
☀️ solar forecast keeps "expected power at point in time" format. Time intervals are interpolated to 15m if needed.
* CO2 assumption: power production landscape follows the market price signals in general. That's why we dont interpolate. In reality it might fluctuate (wind, solar). This has to be solved by higher resolution (15min) co2 input source
TODOs
To be decided
Out of scope