Skip to content

Commit 7a0361f

Browse files
Fix formula precedence bug in the FormulaEngine (#141)
Formulas with repeated operators like `#1 - #2 - #3` were getting calculated incorrectly as `#1 - (#2 - #3)`. The correct post-fix notation would be `[#1, #2, -, #3, -]`, but the formula engine was generating this instead: `[#1, #2, #3, -, -]`. This is fixed by flushing the operators from the build stack not only if a previous operator has a higher precedence, but also if a previous operator has the same precedence as the current operator.
2 parents 838c460 + e6afc6a commit 7a0361f

File tree

3 files changed

+60
-2
lines changed

3 files changed

+60
-2
lines changed

RELEASE_NOTES.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,6 @@
1414

1515
## Bug Fixes
1616

17-
<!-- Here goes notable bug fixes that are worth a special mention or explanation -->
17+
- Formulas with repeated operators like `#1 - #2 - #3` were getting
18+
calculated incorrectly as `#1 - (#2 - #3)`. This has been fixed in
19+
https://github.com/frequenz-floss/frequenz-sdk-python/pull/141

src/frequenz/sdk/timeseries/logical_meter/_formula_engine.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ def push_oper(self, oper: str) -> None:
179179
op_prec = _operator_precedence[oper]
180180
while self._build_stack:
181181
prev_step = self._build_stack[-1]
182-
if op_prec <= _operator_precedence[repr(prev_step)]:
182+
if op_prec < _operator_precedence[repr(prev_step)]:
183183
break
184184
if oper == ")" and repr(prev_step) == "(":
185185
self._build_stack.pop()

tests/timeseries/test_formula_engine.py

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,30 @@ async def test_simple(self) -> None:
122122
([15.0, 20.0, 20.0], -19.25),
123123
],
124124
)
125+
await self.run_test(
126+
"#2 - #4 - #5",
127+
"[#2, #4, -, #5, -]",
128+
[
129+
([6.0, 12.0, 15.0], -21.0),
130+
([15.0, 20.0, 20.0], -25.0),
131+
],
132+
)
133+
await self.run_test(
134+
"#2 + #4 + #5",
135+
"[#2, #4, +, #5, +]",
136+
[
137+
([6.0, 12.0, 15.0], 33.0),
138+
([15.0, 20.0, 20.0], 55.0),
139+
],
140+
)
141+
await self.run_test(
142+
"#2 / #4 / #5",
143+
"[#2, #4, /, #5, /]",
144+
[
145+
([30.0, 3.0, 5.0], 2.0),
146+
([15.0, 3.0, 2.0], 2.5),
147+
],
148+
)
125149

126150
async def test_compound(self) -> None:
127151
"""Test compound formulas."""
@@ -141,6 +165,38 @@ async def test_compound(self) -> None:
141165
([15.0, 17.0, 20.0, 1.5], 10.5),
142166
],
143167
)
168+
await self.run_test(
169+
"#2 + (#4 - #5 * #6)",
170+
"[#2, #4, #5, #6, *, -, +]",
171+
[
172+
([10.0, 12.0, 15.0, 2.0], -8.0),
173+
([15.0, 17.0, 20.0, 1.5], 2.0),
174+
],
175+
)
176+
await self.run_test(
177+
"#2 + (#4 - #5 - #6)",
178+
"[#2, #4, #5, -, #6, -, +]",
179+
[
180+
([10.0, 12.0, 15.0, 2.0], 5.0),
181+
([15.0, 17.0, 20.0, 1.5], 10.5),
182+
],
183+
)
184+
await self.run_test(
185+
"#2 + #4 - #5 - #6",
186+
"[#2, #4, #5, -, #6, -, +]",
187+
[
188+
([10.0, 12.0, 15.0, 2.0], 5.0),
189+
([15.0, 17.0, 20.0, 1.5], 10.5),
190+
],
191+
)
192+
await self.run_test(
193+
"#2 + #4 - (#5 - #6)",
194+
"[#2, #4, #5, #6, -, -, +]",
195+
[
196+
([10.0, 12.0, 15.0, 2.0], 9.0),
197+
([15.0, 17.0, 20.0, 1.5], 13.5),
198+
],
199+
)
144200
await self.run_test(
145201
"(#2 + #4 - #5) * #6",
146202
"[#2, #4, #5, -, +, #6, *]",

0 commit comments

Comments
 (0)