From f84c1c3765885eae33c3f3e2c97fde9943aef24b Mon Sep 17 00:00:00 2001 From: Eric Vin Date: Sun, 7 Jul 2024 19:35:23 +0200 Subject: [PATCH 1/9] Better handling of heuristic sampling + alwaysProvidesOrientation stability. --- src/scenic/domains/driving/roads.py | 2 +- src/scenic/syntax/veneer.py | 23 ++++++++++++++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/scenic/domains/driving/roads.py b/src/scenic/domains/driving/roads.py index ff247f8ab..a332f7836 100644 --- a/src/scenic/domains/driving/roads.py +++ b/src/scenic/domains/driving/roads.py @@ -57,7 +57,7 @@ def _toVector(thing: Vectorlike) -> Vector: def _rejectSample(message): - if veneer.isActive(): + if not veneer.allowSampleRejection(): raise InvalidScenarioError(message) else: raise RejectionException(message) diff --git a/src/scenic/syntax/veneer.py b/src/scenic/syntax/veneer.py index af6ce426c..7497daffa 100644 --- a/src/scenic/syntax/veneer.py +++ b/src/scenic/syntax/veneer.py @@ -299,6 +299,7 @@ ### Internals activity = 0 +heuristicSampling = 0 currentScenario = None scenarioStack = [] scenarios = [] @@ -331,6 +332,25 @@ def isActive(): return activity > 0 +def allowSampleRejection(): + """Should a RejectionException() be allowed + + Returning True means a RejectionException is valid. Returning False indicates + a RejectionException() means the scenario is invalid. + """ + return isActive() or heuristicSampling + + +@contextmanager +def enableHeuristicSampling(): + global heuristicSampling + heuristicSampling += 1 + try: + yield + finally: + heuristicSampling -= 1 + + def activate(options, namespace=None): """Activate the veneer when beginning to compile a Scenic module.""" global activity, _globalParameters, lockedParameters, lockedModel, currentScenario @@ -1517,7 +1537,8 @@ def alwaysProvidesOrientation(region): return True else: # TODO improve somehow! try: - sample = region.sample() + with enableHeuristicSampling(): + sample = region.sample() return sample.orientation is not None or sample is nowhere except RejectionException: return False From ef036ddffeb06e8ed6bcf730900d10db6b290d2e Mon Sep 17 00:00:00 2001 From: Eric Vin Date: Wed, 10 Jul 2024 15:10:21 +0200 Subject: [PATCH 2/9] Refactor and added test. --- src/scenic/domains/driving/roads.py | 13 +++---------- src/scenic/syntax/veneer.py | 13 ++++++------- tests/syntax/test_distributions.py | 16 ++++++++++++++++ 3 files changed, 25 insertions(+), 17 deletions(-) diff --git a/src/scenic/domains/driving/roads.py b/src/scenic/domains/driving/roads.py index a332f7836..e5037a75b 100644 --- a/src/scenic/domains/driving/roads.py +++ b/src/scenic/domains/driving/roads.py @@ -42,7 +42,7 @@ import scenic.core.utils as utils from scenic.core.vectors import Orientation, Vector, VectorField import scenic.syntax.veneer as veneer -from scenic.syntax.veneer import verbosePrint +from scenic.syntax.veneer import rejectSample, verbosePrint ## Typing and utilities @@ -56,16 +56,9 @@ def _toVector(thing: Vectorlike) -> Vector: return type_support.toVector(thing) -def _rejectSample(message): - if not veneer.allowSampleRejection(): - raise InvalidScenarioError(message) - else: - raise RejectionException(message) - - def _rejectIfNonexistent(element, name="network element"): if element is None: - _rejectSample(f"requested {name} does not exist") + rejectSample(f"requested {name} does not exist") return element @@ -1219,7 +1212,7 @@ def findElementWithin(distance): message = reject else: message = "requested element does not exist" - _rejectSample(message) + rejectSample(message) return None def _findPointInAll(self, point, things, key=lambda e: e): diff --git a/src/scenic/syntax/veneer.py b/src/scenic/syntax/veneer.py index 7497daffa..808c9e5dc 100644 --- a/src/scenic/syntax/veneer.py +++ b/src/scenic/syntax/veneer.py @@ -332,13 +332,11 @@ def isActive(): return activity > 0 -def allowSampleRejection(): - """Should a RejectionException() be allowed - - Returning True means a RejectionException is valid. Returning False indicates - a RejectionException() means the scenario is invalid. - """ - return isActive() or heuristicSampling +def rejectSample(message): + if not isActive() or heuristicSampling: + raise RejectionException(message) + else: + raise InvalidScenarioError(message) @contextmanager @@ -1428,6 +1426,7 @@ def In(region): pos = Region.uniformPointIn(region) props = {"position": 1} values = {"position": pos} + breakpoint() if alwaysProvidesOrientation(region): props["parentOrientation"] = 3 values["parentOrientation"] = region.orientation[pos] diff --git a/tests/syntax/test_distributions.py b/tests/syntax/test_distributions.py index c7fda04fc..c45572988 100644 --- a/tests/syntax/test_distributions.py +++ b/tests/syntax/test_distributions.py @@ -797,3 +797,19 @@ def test_object_expression(): for i in range(3): scene = sampleScene(scenario, maxIterations=50) assert len(scene.objects) == 3 + + +## Rejection vs Invalid Scenario Errors + + +def test_rejection_invalid(): + with pytest.raises(InvalidScenarioError): + compileScenic( + """ + from scenic.core.distributions import RejectionException + def foo(): + raise RejectionException("foo") + return Vector(1,1,1) + new Object at foo() + """ + ) From ab289470f45ca4f39b7cfa1df32799d3622c3c26 Mon Sep 17 00:00:00 2001 From: Eric Vin Date: Wed, 10 Jul 2024 21:10:26 +0200 Subject: [PATCH 3/9] Removed breakpoint. --- src/scenic/syntax/veneer.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/scenic/syntax/veneer.py b/src/scenic/syntax/veneer.py index 808c9e5dc..93da3e75d 100644 --- a/src/scenic/syntax/veneer.py +++ b/src/scenic/syntax/veneer.py @@ -1426,7 +1426,6 @@ def In(region): pos = Region.uniformPointIn(region) props = {"position": 1} values = {"position": pos} - breakpoint() if alwaysProvidesOrientation(region): props["parentOrientation"] = 3 values["parentOrientation"] = region.orientation[pos] From 4b20e79a5da11a0e5894b918575f2ed6bc42580c Mon Sep 17 00:00:00 2001 From: Eric Vin Date: Thu, 11 Jul 2024 12:03:30 +0200 Subject: [PATCH 4/9] Refactor to handle rejection errors at a higher level. --- src/scenic/domains/driving/roads.py | 5 ++--- src/scenic/syntax/veneer.py | 20 +------------------- tests/domains/driving/test_driving.py | 12 ++++++++++++ 3 files changed, 15 insertions(+), 22 deletions(-) diff --git a/src/scenic/domains/driving/roads.py b/src/scenic/domains/driving/roads.py index e5037a75b..8d4dc4e5d 100644 --- a/src/scenic/domains/driving/roads.py +++ b/src/scenic/domains/driving/roads.py @@ -34,7 +34,6 @@ distributionFunction, distributionMethod, ) -from scenic.core.errors import InvalidScenarioError import scenic.core.geometry as geometry from scenic.core.object_types import Point from scenic.core.regions import PolygonalRegion, PolylineRegion @@ -42,7 +41,7 @@ import scenic.core.utils as utils from scenic.core.vectors import Orientation, Vector, VectorField import scenic.syntax.veneer as veneer -from scenic.syntax.veneer import rejectSample, verbosePrint +from scenic.syntax.veneer import verbosePrint ## Typing and utilities @@ -58,7 +57,7 @@ def _toVector(thing: Vectorlike) -> Vector: def _rejectIfNonexistent(element, name="network element"): if element is None: - rejectSample(f"requested {name} does not exist") + raise RejectionException(f"requested {name} does not exist") return element diff --git a/src/scenic/syntax/veneer.py b/src/scenic/syntax/veneer.py index 93da3e75d..3a905a4d4 100644 --- a/src/scenic/syntax/veneer.py +++ b/src/scenic/syntax/veneer.py @@ -332,23 +332,6 @@ def isActive(): return activity > 0 -def rejectSample(message): - if not isActive() or heuristicSampling: - raise RejectionException(message) - else: - raise InvalidScenarioError(message) - - -@contextmanager -def enableHeuristicSampling(): - global heuristicSampling - heuristicSampling += 1 - try: - yield - finally: - heuristicSampling -= 1 - - def activate(options, namespace=None): """Activate the veneer when beginning to compile a Scenic module.""" global activity, _globalParameters, lockedParameters, lockedModel, currentScenario @@ -1535,8 +1518,7 @@ def alwaysProvidesOrientation(region): return True else: # TODO improve somehow! try: - with enableHeuristicSampling(): - sample = region.sample() + sample = region.sample() return sample.orientation is not None or sample is nowhere except RejectionException: return False diff --git a/tests/domains/driving/test_driving.py b/tests/domains/driving/test_driving.py index 4ea809437..e998cc15d 100644 --- a/tests/domains/driving/test_driving.py +++ b/tests/domains/driving/test_driving.py @@ -5,6 +5,7 @@ import pytest from scenic.core.distributions import RejectionException +from scenic.core.errors import InvalidScenarioError from scenic.core.geometry import TriangulationError from scenic.domains.driving.roads import Network from tests.utils import compileScenic, pickle_test, sampleEgo, sampleScene, tryPickling @@ -206,3 +207,14 @@ def test_pickle(cached_maps): unpickled = tryPickling(scenario) scene = sampleScene(unpickled, maxIterations=1000) tryPickling(scene) + + +def test_invalid_road_scenario(cached_maps): + with pytest.raises(InvalidScenarioError): + scenario = compileDrivingScenario( + cached_maps, + """ + ego = new Car at 80.6354425964952@-327.5431187869811, with color (1,1,1) + param foo = ego.oppositeLaneGroup.sidewalk + """, + ) From ee41376a2844fac117077a72b9cc0de996f1e218 Mon Sep 17 00:00:00 2001 From: Eric Vin Date: Thu, 11 Jul 2024 12:13:45 +0200 Subject: [PATCH 5/9] Added additional test and cleanup. --- src/scenic/domains/driving/roads.py | 2 +- tests/domains/driving/test_driving.py | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/scenic/domains/driving/roads.py b/src/scenic/domains/driving/roads.py index 8d4dc4e5d..48289f0c8 100644 --- a/src/scenic/domains/driving/roads.py +++ b/src/scenic/domains/driving/roads.py @@ -1211,7 +1211,7 @@ def findElementWithin(distance): message = reject else: message = "requested element does not exist" - rejectSample(message) + raise RejectionException(message) return None def _findPointInAll(self, point, things, key=lambda e: e): diff --git a/tests/domains/driving/test_driving.py b/tests/domains/driving/test_driving.py index e998cc15d..8eab12f64 100644 --- a/tests/domains/driving/test_driving.py +++ b/tests/domains/driving/test_driving.py @@ -218,3 +218,15 @@ def test_invalid_road_scenario(cached_maps): param foo = ego.oppositeLaneGroup.sidewalk """, ) + + with pytest.raises(InvalidScenarioError): + # Set regionContainedIn to everywhere to hit driving domain specific code + # instead of high level not contained in workspace rejection. + scenario = compileDrivingScenario( + cached_maps, + """ + ego = new Car at 10000@10000, + with color (1,1,1), with regionContainedIn everywhere + param foo = ego.lane + """, + ) From 209f53031af437e1c9cfb07e8fa6c2fb1231473b Mon Sep 17 00:00:00 2001 From: Eric Vin Date: Thu, 11 Jul 2024 12:15:00 +0200 Subject: [PATCH 6/9] Additional cleanup. --- src/scenic/syntax/veneer.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/scenic/syntax/veneer.py b/src/scenic/syntax/veneer.py index 3a905a4d4..af6ce426c 100644 --- a/src/scenic/syntax/veneer.py +++ b/src/scenic/syntax/veneer.py @@ -299,7 +299,6 @@ ### Internals activity = 0 -heuristicSampling = 0 currentScenario = None scenarioStack = [] scenarios = [] From d20fe1c805c1d81c05bb72987042f417ea675c8e Mon Sep 17 00:00:00 2001 From: Eric Vin Date: Thu, 11 Jul 2024 12:18:08 +0200 Subject: [PATCH 7/9] Added additional comment. --- tests/domains/driving/test_driving.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/domains/driving/test_driving.py b/tests/domains/driving/test_driving.py index 8eab12f64..4eb26eb5f 100644 --- a/tests/domains/driving/test_driving.py +++ b/tests/domains/driving/test_driving.py @@ -211,6 +211,9 @@ def test_pickle(cached_maps): def test_invalid_road_scenario(cached_maps): with pytest.raises(InvalidScenarioError): + # Set color to fixed value to avoid creating a distribution and allow + # us to detect the invalid scenario at compile time. + # TODO: Can we remove this limitation? scenario = compileDrivingScenario( cached_maps, """ From b91592152ddbe6c4a89eca3e90bd9be4a0d47eed Mon Sep 17 00:00:00 2001 From: Eric Vin Date: Fri, 12 Jul 2024 19:21:20 +0200 Subject: [PATCH 8/9] Car method improvements. --- src/scenic/domains/driving/model.scenic | 28 ++++++++++++------------- tests/domains/driving/test_driving.py | 5 ++--- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/src/scenic/domains/driving/model.scenic b/src/scenic/domains/driving/model.scenic index b6732d2fb..988d03072 100644 --- a/src/scenic/domains/driving/model.scenic +++ b/src/scenic/domains/driving/model.scenic @@ -146,12 +146,12 @@ class DrivingObject: The simulation is rejected if the object is not in a lane. (Use `DrivingObject._lane` to get `None` instead.) """ - return network.laneAt(self, reject='object is not in a lane') + return network.laneAt(self.position, reject='object is not in a lane') @property def _lane(self) -> Optional[Lane]: """The `Lane` at the object's current position, if any.""" - return network.laneAt(self) + return network.laneAt(self.position) @property def laneSection(self) -> LaneSection: @@ -159,12 +159,12 @@ class DrivingObject: The simulation is rejected if the object is not in a lane. """ - return network.laneSectionAt(self, reject='object is not in a lane') + return network.laneSectionAt(self.position, reject='object is not in a lane') @property def _laneSection(self) -> Optional[LaneSection]: """The `LaneSection` at the object's current position, if any.""" - return network.laneSectionAt(self) + return network.laneSectionAt(self.position) @property def laneGroup(self) -> LaneGroup: @@ -172,12 +172,12 @@ class DrivingObject: The simulation is rejected if the object is not in a lane. """ - return network.laneGroupAt(self, reject='object is not in a lane') + return network.laneGroupAt(self.position, reject='object is not in a lane') @property def _laneGroup(self) -> Optional[LaneGroup]: """The `LaneGroup` at the object's current position, if any.""" - return network.laneGroupAt(self) + return network.laneGroupAt(self.position) @property def oppositeLaneGroup(self) -> LaneGroup: @@ -193,12 +193,12 @@ class DrivingObject: The simulation is rejected if the object is not on a road. """ - return network.roadAt(self, reject='object is not on a road') + return network.roadAt(self.position, reject='object is not on a road') @property def _road(self) -> Optional[Road]: """The `Road` at the object's current position, if any.""" - return network.roadAt(self) + return network.roadAt(self.position) @property def intersection(self) -> Intersection: @@ -206,12 +206,12 @@ class DrivingObject: The simulation is rejected if the object is not in an intersection. """ - return network.intersectionAt(self, reject='object is not in an intersection') + return network.intersectionAt(self.position, reject='object is not in an intersection') @property def _intersection(self) -> Optional[Intersection]: """The `Intersection` at the object's current position, if any.""" - return network.intersectionAt(self) + return network.intersectionAt(self.position) @property def crossing(self) -> PedestrianCrossing: @@ -219,12 +219,12 @@ class DrivingObject: The simulation is rejected if the object is not in a crosswalk. """ - return network.crossingAt(self, reject='object is not in a crossing') + return network.crossingAt(self.position, reject='object is not in a crossing') @property def _crossing(self) -> Optional[PedestrianCrossing]: """The `PedestrianCrossing` at the object's current position, if any.""" - return network.crossingAt(self) + return network.crossingAt(self.position) @property def element(self) -> NetworkElement: @@ -233,12 +233,12 @@ class DrivingObject: See `Network.elementAt` for the details of how this is determined. The simulation is rejected if the object is not in any network element. """ - return network.elementAt(self, reject='object is not on any network element') + return network.elementAt(self.position, reject='object is not on any network element') @property def _element(self) -> Optional[NetworkElement]: """The highest-level `NetworkElement` at the object's current position, if any.""" - return network.elementAt(self) + return network.elementAt(self.position) # Utility functions diff --git a/tests/domains/driving/test_driving.py b/tests/domains/driving/test_driving.py index 4eb26eb5f..29b8a8b7b 100644 --- a/tests/domains/driving/test_driving.py +++ b/tests/domains/driving/test_driving.py @@ -217,7 +217,7 @@ def test_invalid_road_scenario(cached_maps): scenario = compileDrivingScenario( cached_maps, """ - ego = new Car at 80.6354425964952@-327.5431187869811, with color (1,1,1) + ego = new Car at 80.6354425964952@-327.5431187869811 param foo = ego.oppositeLaneGroup.sidewalk """, ) @@ -228,8 +228,7 @@ def test_invalid_road_scenario(cached_maps): scenario = compileDrivingScenario( cached_maps, """ - ego = new Car at 10000@10000, - with color (1,1,1), with regionContainedIn everywhere + ego = new Car at 10000@10000, with regionContainedIn everywhere param foo = ego.lane """, ) From 275948875dd134c406d081af087e7c0f5a4a1046 Mon Sep 17 00:00:00 2001 From: Eric Vin Date: Wed, 17 Jul 2024 16:51:13 -0700 Subject: [PATCH 9/9] Removed old comment --- tests/domains/driving/test_driving.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/domains/driving/test_driving.py b/tests/domains/driving/test_driving.py index 29b8a8b7b..50d09266d 100644 --- a/tests/domains/driving/test_driving.py +++ b/tests/domains/driving/test_driving.py @@ -211,9 +211,6 @@ def test_pickle(cached_maps): def test_invalid_road_scenario(cached_maps): with pytest.raises(InvalidScenarioError): - # Set color to fixed value to avoid creating a distribution and allow - # us to detect the invalid scenario at compile time. - # TODO: Can we remove this limitation? scenario = compileDrivingScenario( cached_maps, """