Skip to content

Commit c11237d

Browse files
anand-balaanand-bala
committed
fix: Next and Prev evaluation should properly use steps
Closes #3
1 parent 99df237 commit c11237d

2 files changed

Lines changed: 357 additions & 7 deletions

File tree

‎csrc/monitoring/evaluation.cc‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -271,9 +271,9 @@ template <std::bidirectional_iterator Iter, std::sized_sentinel_for<Iter> Sentin
271271
template <std::bidirectional_iterator Iter, std::sized_sentinel_for<Iter> Sentinel = Iter>
272272
[[nodiscard]] auto eval_next(const stql::NextExpr& e, const EvaluationContext<Iter, Sentinel>& ctx)
273273
-> bool {
274-
// Check if there is any next frame
275-
if (!ctx.has_horizon()) { return false; }
276-
EvaluationContext<Iter, Sentinel> next_ctx = shift_eval_context(ctx, 1);
274+
// Check if the frame `e.steps` away is available
275+
if (static_cast<size_t>(ctx.num_horizon()) < e.steps) { return false; }
276+
EvaluationContext<Iter, Sentinel> next_ctx = shift_eval_context(ctx, e.steps);
277277
return eval_impl(*e.arg, next_ctx);
278278
}
279279

@@ -403,11 +403,11 @@ eval_release(const stql::ReleaseExpr& e, const EvaluationContext<Iter, Sentinel>
403403
template <std::bidirectional_iterator Iter, std::sized_sentinel_for<Iter> Sentinel = Iter>
404404
[[nodiscard]] auto
405405
eval_previous(const stql::PreviousExpr& e, const EvaluationContext<Iter, Sentinel>& ctx) -> bool {
406-
// No previous frame available: vacuously false
407-
if (!ctx.has_history()) { return false; }
408-
406+
// Check if the frame `e.steps` away is available
407+
if (static_cast<size_t>(ctx.num_history()) < e.steps) { return false; }
408+
const auto neg_shift = -1 * static_cast<int64_t>(e.steps);
409409
// Create context for previous frame (most recent in history)
410-
EvaluationContext<Iter, Sentinel> prev_ctx = shift_eval_context(ctx, -1);
410+
EvaluationContext<Iter, Sentinel> prev_ctx = shift_eval_context(ctx, neg_shift);
411411
return eval_impl(*e.arg, prev_ctx);
412412
}
413413

‎tests/test_boolean_evaluation.cc‎

Lines changed: 350 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,125 @@ TEST_CASE("BooleanEvaluator - Future-Time Temporal Operators", "[evaluation][tem
239239
REQUIRE(evaluator.evaluate(formula, frame1, empty_history, horizon));
240240
}
241241

242+
// ==========================================================================
243+
// NextExpr Tests with Varying Steps
244+
// ==========================================================================
245+
246+
SECTION("NextExpr - steps=2 evaluates 2 frames ahead") {
247+
// Create frames: current (no car), +1 (no car), +2 (has car)
248+
auto frame0 = make_empty_frame(0, 0.0);
249+
auto frame1 = make_empty_frame(1, 1.0);
250+
auto frame2 = make_frame_with_car(2, 2.0);
251+
252+
std::deque<datastream::Frame> horizon;
253+
horizon.push_back(frame1);
254+
horizon.push_back(frame2);
255+
256+
// Formula: next(2, exists car)
257+
auto car_id = ObjectVar{"car"};
258+
auto is_car = is_class(car_id, CAR);
259+
auto condition = exists({car_id}, is_car);
260+
auto formula = next(condition, 2);
261+
262+
// Should be TRUE: car exists at frame +2
263+
REQUIRE(evaluator.evaluate(formula, frame0, empty_history, horizon));
264+
}
265+
266+
SECTION("NextExpr - steps=2 fails when condition false at target frame") {
267+
// Create frames: current (no car), +1 (has car), +2 (no car)
268+
auto frame0 = make_empty_frame(0, 0.0);
269+
auto frame1 = make_frame_with_car(1, 1.0);
270+
auto frame2 = make_empty_frame(2, 2.0);
271+
272+
std::deque<datastream::Frame> horizon;
273+
horizon.push_back(frame1);
274+
horizon.push_back(frame2);
275+
276+
// Formula: next(2, exists car)
277+
auto car_id = ObjectVar{"car"};
278+
auto is_car = is_class(car_id, CAR);
279+
auto condition = exists({car_id}, is_car);
280+
auto formula = next(condition, 2);
281+
282+
// Should be FALSE: car doesn't exist at frame +2
283+
REQUIRE_FALSE(evaluator.evaluate(formula, frame0, empty_history, horizon));
284+
}
285+
286+
SECTION("NextExpr - steps=5 evaluates 5 frames ahead") {
287+
// Create frames: current + 5 in horizon
288+
auto frame0 = make_empty_frame(0, 0.0);
289+
std::deque<datastream::Frame> horizon;
290+
horizon.push_back(make_empty_frame(1, 1.0));
291+
horizon.push_back(make_empty_frame(2, 2.0));
292+
horizon.push_back(make_frame_with_person(3, 3.0)); // Person at +3
293+
horizon.push_back(make_empty_frame(4, 4.0));
294+
horizon.push_back(make_frame_with_car(5, 5.0)); // Car at +5
295+
296+
// Formula: next(5, exists car)
297+
auto car_id = ObjectVar{"car"};
298+
auto is_car = is_class(car_id, CAR);
299+
auto condition = exists({car_id}, is_car);
300+
auto formula = next(condition, 5);
301+
302+
// Should be TRUE: car exists at frame +5
303+
REQUIRE(evaluator.evaluate(formula, frame0, empty_history, horizon));
304+
}
305+
306+
SECTION("NextExpr - steps=3 with person detection") {
307+
// Create frames with person appearing at +3
308+
auto frame0 = make_empty_frame(0, 0.0);
309+
std::deque<datastream::Frame> horizon;
310+
horizon.push_back(make_frame_with_car(1, 1.0)); // Car at +1
311+
horizon.push_back(make_frame_with_car(2, 2.0)); // Car at +2
312+
horizon.push_back(make_frame_with_person(3, 3.0)); // Person at +3
313+
314+
// Formula: next(3, exists person)
315+
auto person_id = ObjectVar{"person"};
316+
auto is_person = is_class(person_id, PERSON);
317+
auto condition = exists({person_id}, is_person);
318+
auto formula = next(condition, 3);
319+
320+
// Should be TRUE: person exists at frame +3
321+
REQUIRE(evaluator.evaluate(formula, frame0, empty_history, horizon));
322+
}
323+
324+
SECTION("NextExpr - steps=4 fails when insufficient horizon") {
325+
// Only 3 frames in horizon, but asking for +4
326+
auto frame0 = make_empty_frame(0, 0.0);
327+
std::deque<datastream::Frame> horizon;
328+
horizon.push_back(make_empty_frame(1, 1.0));
329+
horizon.push_back(make_empty_frame(2, 2.0));
330+
horizon.push_back(make_frame_with_car(3, 3.0));
331+
332+
// Formula: next(4, true)
333+
auto formula = next(make_true(), 4);
334+
335+
// Should be FALSE: not enough frames in horizon
336+
REQUIRE_FALSE(evaluator.evaluate(formula, frame0, empty_history, horizon));
337+
}
338+
339+
SECTION("NextExpr - nested with different steps") {
340+
// Test: next(2, next(3, condition))
341+
// This should evaluate at frame +5 (2 + 3)
342+
auto frame0 = make_empty_frame(0, 0.0);
343+
std::deque<datastream::Frame> horizon;
344+
horizon.push_back(make_empty_frame(1, 1.0));
345+
horizon.push_back(make_empty_frame(2, 2.0));
346+
horizon.push_back(make_empty_frame(3, 3.0));
347+
horizon.push_back(make_empty_frame(4, 4.0));
348+
horizon.push_back(make_frame_with_car(5, 5.0)); // Car at +5
349+
350+
// Formula: next(2, next(3, exists car))
351+
auto car_id = ObjectVar{"car"};
352+
auto is_car = is_class(car_id, CAR);
353+
auto condition = exists({car_id}, is_car);
354+
auto inner = next(condition, 3);
355+
auto formula = next(inner, 2);
356+
357+
// Should be TRUE: evaluates at frame +2+3=+5 where car exists
358+
REQUIRE(evaluator.evaluate(formula, frame0, empty_history, horizon));
359+
}
360+
242361
// ==========================================================================
243362
// AlwaysExpr Tests
244363
// ==========================================================================
@@ -538,6 +657,237 @@ TEST_CASE("BooleanEvaluator - Past-Time Temporal Operators", "[evaluation][tempo
538657
REQUIRE(evaluator.evaluate(formula, frame, history, empty_horizon));
539658
}
540659

660+
// ==========================================================================
661+
// PreviousExpr Tests with Varying Steps
662+
// ==========================================================================
663+
664+
SECTION("PreviousExpr - steps=2 evaluates 2 frames back") {
665+
// Create frames: -2 (has car), -1 (no car), current (no car)
666+
auto frame_current = make_empty_frame(2, 2.0);
667+
std::deque<datastream::Frame> history;
668+
history.push_back(make_frame_with_car(0, 0.0)); // Frame at -2
669+
history.push_back(make_empty_frame(1, 1.0)); // Frame at -1
670+
671+
// Formula: previous(2, exists car)
672+
auto car_id = ObjectVar{"car"};
673+
auto is_car = is_class(car_id, CAR);
674+
auto condition = exists({car_id}, is_car);
675+
auto formula = previous(condition, 2);
676+
677+
// Should be TRUE: car exists at frame -2
678+
REQUIRE(evaluator.evaluate(formula, frame_current, history, empty_horizon));
679+
}
680+
681+
SECTION("PreviousExpr - steps=2 fails when condition false at target frame") {
682+
// Create frames: -2 (no car), -1 (has car), current (no car)
683+
auto frame_current = make_empty_frame(2, 2.0);
684+
std::deque<datastream::Frame> history;
685+
history.push_back(make_empty_frame(0, 0.0)); // Frame at -2
686+
history.push_back(make_frame_with_car(1, 1.0)); // Frame at -1
687+
688+
// Formula: previous(2, exists car)
689+
auto car_id = ObjectVar{"car"};
690+
auto is_car = is_class(car_id, CAR);
691+
auto condition = exists({car_id}, is_car);
692+
auto formula = previous(condition, 2);
693+
694+
// Should be FALSE: car doesn't exist at frame -2
695+
REQUIRE_FALSE(evaluator.evaluate(formula, frame_current, history, empty_horizon));
696+
}
697+
698+
SECTION("PreviousExpr - steps=5 evaluates 5 frames back") {
699+
// Create frames: 5 frames of history + current
700+
auto frame_current = make_empty_frame(5, 5.0);
701+
std::deque<datastream::Frame> history;
702+
history.push_back(make_frame_with_car(0, 0.0)); // Frame at -5 (has car)
703+
history.push_back(make_empty_frame(1, 1.0)); // Frame at -4
704+
history.push_back(make_frame_with_person(2, 2.0)); // Frame at -3 (has person)
705+
history.push_back(make_empty_frame(3, 3.0)); // Frame at -2
706+
history.push_back(make_empty_frame(4, 4.0)); // Frame at -1
707+
708+
// Formula: previous(5, exists car)
709+
auto car_id = ObjectVar{"car"};
710+
auto is_car = is_class(car_id, CAR);
711+
auto condition = exists({car_id}, is_car);
712+
auto formula = previous(condition, 5);
713+
714+
// Should be TRUE: car exists at frame -5
715+
REQUIRE(evaluator.evaluate(formula, frame_current, history, empty_horizon));
716+
}
717+
718+
SECTION("PreviousExpr - steps=3 with person detection") {
719+
// Create frames with person appearing at -3
720+
auto frame_current = make_empty_frame(3, 3.0);
721+
std::deque<datastream::Frame> history;
722+
history.push_back(make_frame_with_person(0, 0.0)); // Person at -3
723+
history.push_back(make_frame_with_car(1, 1.0)); // Car at -2
724+
history.push_back(make_frame_with_car(2, 2.0)); // Car at -1
725+
726+
// Formula: previous(3, exists person)
727+
auto person_id = ObjectVar{"person"};
728+
auto is_person = is_class(person_id, PERSON);
729+
auto condition = exists({person_id}, is_person);
730+
auto formula = previous(condition, 3);
731+
732+
// Should be TRUE: person exists at frame -3
733+
REQUIRE(evaluator.evaluate(formula, frame_current, history, empty_horizon));
734+
}
735+
736+
SECTION("PreviousExpr - steps=4 fails when insufficient history") {
737+
// Only 3 frames in history, but asking for -4
738+
auto frame_current = make_empty_frame(3, 3.0);
739+
std::deque<datastream::Frame> history;
740+
history.push_back(make_empty_frame(0, 0.0));
741+
history.push_back(make_empty_frame(1, 1.0));
742+
history.push_back(make_frame_with_car(2, 2.0));
743+
744+
// Formula: previous(4, true)
745+
auto formula = previous(make_true(), 4);
746+
747+
// Should be FALSE: not enough frames in history
748+
REQUIRE_FALSE(evaluator.evaluate(formula, frame_current, history, empty_horizon));
749+
}
750+
751+
SECTION("PreviousExpr - nested with different steps") {
752+
// Test: previous(2, previous(3, condition))
753+
// This should evaluate at frame -5 (2 + 3)
754+
auto frame_current = make_empty_frame(5, 5.0);
755+
std::deque<datastream::Frame> history;
756+
history.push_back(make_frame_with_car(0, 0.0)); // Car at -5
757+
history.push_back(make_empty_frame(1, 1.0)); // Frame at -4
758+
history.push_back(make_empty_frame(2, 2.0)); // Frame at -3
759+
history.push_back(make_empty_frame(3, 3.0)); // Frame at -2
760+
history.push_back(make_empty_frame(4, 4.0)); // Frame at -1
761+
762+
// Formula: previous(2, previous(3, exists car))
763+
auto car_id = ObjectVar{"car"};
764+
auto is_car = is_class(car_id, CAR);
765+
auto condition = exists({car_id}, is_car);
766+
auto inner = previous(condition, 3);
767+
auto formula = previous(inner, 2);
768+
769+
// Should be TRUE: evaluates at frame -2-3=-5 where car exists
770+
REQUIRE(evaluator.evaluate(formula, frame_current, history, empty_horizon));
771+
}
772+
773+
// ==========================================================================
774+
// Integration Tests: NextExpr/PreviousExpr with Multiple Steps
775+
// ==========================================================================
776+
777+
SECTION("NextExpr - steps with quantifier and class comparison") {
778+
// Verify that object bindings work correctly at distant frames
779+
auto frame0 = make_empty_frame(0, 0.0);
780+
std::deque<datastream::Frame> horizon;
781+
horizon.push_back(make_empty_frame(1, 1.0));
782+
horizon.push_back(make_frame_with_multiple_objects(2, 2.0)); // Has both car and person
783+
horizon.push_back(make_empty_frame(3, 3.0));
784+
785+
// Formula: next(2, exists obj such that obj is a car)
786+
auto obj_id = ObjectVar{"obj"};
787+
auto is_car = is_class(obj_id, CAR);
788+
auto condition = exists({obj_id}, is_car);
789+
auto formula = next(condition, 2);
790+
791+
// Should be TRUE: car exists at frame +2
792+
REQUIRE(evaluator.evaluate(formula, frame0, {}, horizon));
793+
}
794+
795+
SECTION("PreviousExpr - steps with quantifier and class comparison") {
796+
// Verify that object bindings work correctly at distant past frames
797+
auto frame_current = make_empty_frame(3, 3.0);
798+
std::deque<datastream::Frame> history;
799+
history.push_back(make_empty_frame(0, 0.0));
800+
history.push_back(make_frame_with_multiple_objects(1, 1.0)); // Has both car and person
801+
history.push_back(make_empty_frame(2, 2.0));
802+
803+
// Formula: previous(2, exists obj such that obj is a person)
804+
auto obj_id = ObjectVar{"obj"};
805+
auto is_person = is_class(obj_id, PERSON);
806+
auto condition = exists({obj_id}, is_person);
807+
auto formula = previous(condition, 2);
808+
809+
// Should be TRUE: person exists at frame -2
810+
REQUIRE(evaluator.evaluate(formula, frame_current, history, {}));
811+
}
812+
813+
SECTION("NextExpr - large steps with probability check") {
814+
// Test with larger step count and probability comparison
815+
auto frame0 = make_empty_frame(0, 0.0);
816+
std::deque<datastream::Frame> horizon;
817+
for (int i = 1; i <= 9; ++i) {
818+
horizon.push_back(make_empty_frame(i, static_cast<double>(i)));
819+
}
820+
horizon.push_back(make_frame_with_car_probability(10, 10.0, 0.99)); // High confidence at +10
821+
822+
// Formula: next(10, exists car with P(car) >= 0.95)
823+
auto car_id = ObjectVar{"car"};
824+
auto is_car = is_class(car_id, CAR);
825+
auto high_prob = high_confidence(car_id, 0.95);
826+
auto condition = exists({car_id}, is_car & high_prob);
827+
auto formula = next(condition, 10);
828+
829+
// Should be TRUE: high-confidence car at frame +10
830+
REQUIRE(evaluator.evaluate(formula, frame0, {}, horizon));
831+
}
832+
833+
SECTION("PreviousExpr - large steps with multiple object types") {
834+
// Test with larger step count going backwards
835+
auto frame_current = make_empty_frame(10, 10.0);
836+
std::deque<datastream::Frame> history;
837+
history.push_back(make_frame_with_multiple_objects(0, 0.0)); // Both car and person at -10
838+
for (int i = 1; i < 10; ++i) {
839+
history.push_back(make_empty_frame(i, static_cast<double>(i)));
840+
}
841+
842+
// Formula: previous(10, exists both car and person)
843+
auto car_id = ObjectVar{"car"};
844+
auto person_id = ObjectVar{"person"};
845+
auto is_car = is_class(car_id, CAR);
846+
auto is_person = is_class(person_id, PERSON);
847+
auto has_car = exists({car_id}, is_car);
848+
auto has_person = exists({person_id}, is_person);
849+
auto condition = has_car & has_person;
850+
auto formula = previous(condition, 10);
851+
852+
// Should be TRUE: both objects exist at frame -10
853+
REQUIRE(evaluator.evaluate(formula, frame_current, history, {}));
854+
}
855+
856+
SECTION("NextExpr and PreviousExpr - asymmetric evaluation") {
857+
// Verify that next(N) and previous(N) evaluate at correct frames
858+
// Setup: -2, -1, current, +1, +2
859+
// Only frame -2 and +2 have cars
860+
auto frame_current = make_empty_frame(2, 2.0);
861+
862+
std::deque<datastream::Frame> history;
863+
history.push_back(make_frame_with_car(0, 0.0)); // Frame -2: has car
864+
history.push_back(make_empty_frame(1, 1.0)); // Frame -1: no car
865+
866+
std::deque<datastream::Frame> horizon;
867+
horizon.push_back(make_empty_frame(3, 3.0)); // Frame +1: no car
868+
horizon.push_back(make_frame_with_car(4, 4.0)); // Frame +2: has car
869+
870+
auto car_id = ObjectVar{"car"};
871+
auto is_car = is_class(car_id, CAR);
872+
auto condition = exists({car_id}, is_car);
873+
874+
// next(2, condition) should be TRUE (car at +2)
875+
auto next_formula = next(condition, 2);
876+
REQUIRE(evaluator.evaluate(next_formula, frame_current, history, horizon));
877+
878+
// previous(2, condition) should be TRUE (car at -2)
879+
auto prev_formula = previous(condition, 2);
880+
REQUIRE(evaluator.evaluate(prev_formula, frame_current, history, horizon));
881+
882+
// next(1, condition) should be FALSE (no car at +1)
883+
auto next_1_formula = next(condition, 1);
884+
REQUIRE_FALSE(evaluator.evaluate(next_1_formula, frame_current, history, horizon));
885+
886+
// previous(1, condition) should be FALSE (no car at -1)
887+
auto prev_1_formula = previous(condition, 1);
888+
REQUIRE_FALSE(evaluator.evaluate(prev_1_formula, frame_current, history, horizon));
889+
}
890+
541891
// ==========================================================================
542892
// HoldsExpr Tests
543893
// ==========================================================================

0 commit comments

Comments
 (0)