Add dynamic rulebook code and example by kevinchang73 · Pull Request #59 · BerkeleyLearnVerify/VerifAI

kevinchang73 · 2025-12-02T01:24:10Z

No description provided.

dfremont · 2026-04-17T18:42:07Z

Is the example really specific to Town05? If not, can we change it to use Town01 so that it's not necessary to include another map in the repo? (Town01 is already under the tests folder)

dfremont · 2026-04-17T18:44:59Z

+bypass_dist_max = []
+
+for file in all_files:
+    infile = open(directory+'/'+file, 'r')


Use a with statement so that the file is properly closed.

Also, hard-coding / will not work on Windows: use pathlib or os.path to manipulate paths. (Actually in this particular case glob.glob would probably be best, since then you don't need to manually use endswith and startswith above.)

dfremont · 2026-04-17T18:45:51Z

+import sys
+import matplotlib.pyplot as plt
+import numpy as np
+import os


In all files, please group imports into 3 blocks separated by 1 blank line: Python stdlib, external packages, local imports from VerifAI itself.

dfremont · 2026-04-17T18:47:40Z

+
+print('Error weights')
+print('segment 0:')
+for i in range(1):


Why these unnecessary loops? If this code is copied from a template, why not expose the general template as a helper function which you can import here?

dfremont · 2026-04-17T18:50:50Z

+#################################
+
+behavior EgoBehavior(path):
+    current_lane = network.laneAt(self)


Suggested change

current_lane = network.laneAt(self)

current_lane = self.lane

It doesn't look like you're handling the case when the car is not in a lane, so I've used the lane property here, which will reject the simulation if the car isn't in a lane. If you actually want current_lane to be None in that case, use the _lane property instead (documentation here).

dfremont · 2026-04-17T18:51:29Z

+        do FollowLaneBehavior(globalParameters.EGO_SPEED, is_oppositeTraffic=True) until (distance to blockingCar) > globalParameters.BYPASS_DIST
+        laneChangeCompleted = True
+    interrupt when (blockingCar can see ego) and (distance to blockingCar) > globalParameters.BYPASS_DIST and not bypassed:
+        current_laneSection = network.laneSectionAt(self)


Suggested change

current_laneSection = network.laneSectionAt(self)

current_laneSection = self.laneSection

dfremont · 2026-04-17T18:52:34Z

+require (distance from blockingCar to intersection) > DIST_TO_INTERSECTION
+terminate when (distance to spawnPt) > TERM_DIST


Suggested change

require (distance from blockingCar to intersection) > DIST_TO_INTERSECTION

terminate when (distance to spawnPt) > TERM_DIST

require distance from blockingCar to intersection > DIST_TO_INTERSECTION

terminate when distance to spawnPt > TERM_DIST

dfremont · 2026-04-17T18:53:31Z

+# RECORDING                     #
+#################################
+
+record initial (initLaneSec.polygon.exterior.coords) as initLaneCoords


Likewise, the parser should be able to handle having no parentheses here.

dfremont · 2026-04-17T19:00:29Z

@@ -0,0 +1,19 @@
+import numpy as np
+
+def rule0(simulation, indices): # safe distance to obstacle


Did you decide not to implement my suggestion for a decorator to reduce the boilerplate here?

dfremont · 2026-04-17T19:03:59Z

+# AGENT BEHAVIORS               #
+#################################
+
+behavior DecelerateBehavior(brake):


What's the purpose of having a behavior that only takes 1 action? Why not just inline the body?

dfremont · 2026-04-17T19:17:30Z

+    try:
+        do FollowTrajectoryBehavior(target_speed=globalParameters.EGO_SPEED, trajectory=trajectory)
+        do FollowLaneBehavior(target_speed=globalParameters.ADV_SPEED)
+    interrupt when withinDistanceToAnyObjs(self, SAFETY_DIST) and (ped in network.drivableRegion) and flag:


Is the intended behavior here that once this interrupt fires the first time, it can never fire again because the flag is set to false? That's a little surprising, so it might be good to explain what the ego is trying to do either here or in the docstring for the whole scenario (which is just a stub).

dfremont · 2026-04-17T19:20:04Z

+
+ped = new Pedestrian at pedSpawnPt,
+    facing toward pedEndPt,
+    with regionContainedIn None,


This is the default (meaning the pedestrian can be anywhere in the workspace). Did you mean everywhere?

dfremont · 2026-04-17T19:22:17Z

@@ -0,0 +1,137 @@
+"""
+TITLE: Verifai 2.0 Left Turn


Suggested change

TITLE: Verifai 2.0 Left Turn

TITLE: VerifAI 3.0 Left Turn

dfremont · 2026-04-17T19:34:53Z

+record (distance from ego to adv1) as egoDistToAdv1
+record (distance to egoSpawnPt) as egoDistToEgoSpawnPt
+
+record ego._boundingPolygon as egoPoly


I don't think we should be using an internal, undocumented property in our examples. You can use ego.boundingPolygon to get a PolygonalRegion, which should be fine for whatever you need to do with it.

Actually having said that, it doesn't seem that these polygons are used anywhere. So I would remove them.

dfremont · 2026-04-17T19:40:08Z

+        for root, _, files in os.walk(path):
+            for name in files:
+                fname = os.path.join(root, name)
+                if os.path.splitext(fname)[1] == '.scenic':
+                    paths.append(fname)


I think something like glob.glob(f"{path}/**/*.scenic") would work just as well here and be a lot easier to read.

dfremont · 2026-04-17T19:40:26Z

+    for p in paths:
+        falsifier = run_experiment(p, rulebook=rulebook, 
+        parallel=parallel, model=model, sampler_type=sampler_type, headless=headless,
+        num_workers=num_workers, max_time=max_time, n_iters=n_iters, max_steps=max_steps)


Confusing indentation

dfremont · 2026-04-17T19:41:16Z

+    model: Which simulator model to use (e.g. scenic.simulators.newtonian.driving_model)
+    sampler_type: Which VerifAI sampelr to use (e.g. halton, scenic, ce, mab, etc.)
+    headless: Whether or not to display each simulation.
+    num_workers: Number of parallel workers. Only used if parallel is true.


Missing some arguments here

dfremont · 2026-04-17T19:42:17Z

+    print(f'(multi.py) Sampler type: {falsifier.sampler_type}')
+
+    # Run falsification
+    t0 = time.time()


All timings should use time.monotonic() so as not to be affected by changes to the system clock.

dfremont · 2026-04-17T19:43:24Z

+if __name__ == '__main__':
+    parser = argparse.ArgumentParser()
+    parser.add_argument('--scenic-path', '-sp', type=str, default='uberCrashNewton.scenic',
+    help='Path to Scenic script')


Confusing indentation (please go through all files for this kind of thing, or run a tool like Black)

dfremont · 2026-04-17T19:45:51Z

+git clone https://github.com/BerkeleyLearnVerify/VerifAI
+cd VerifAI
+python -m pip install --upgrade pip
+python -m pip install -e .


Suggested change

python -m pip install -e .

python -m pip install -e ".[examples]"

dfremont · 2026-04-17T19:46:52Z

+python -m pip install -e .
+```
+
+Then, for this example, we adopt the [Metadrive simulator](https://metadriverse.github.io/metadrive/) as the backend simulator. To install Metadrive, run the following command:


You should just add MetaDrive and sumolib to the examples extra instead of requiring a separate pip invocation (which can break dependency constraints).

dfremont · 2026-04-17T19:48:23Z

+
+```bash
+python -m pip uninstall progressbar
+python -m pip install --force-reinstall progressbar2==3.55.0


This is not a good idea. Is it hard to fix our code to use progressbar2? I would hope it would be a simple change.

dfremont · 2026-04-17T19:50:47Z

+
+We provide six different scenarios in the `examples/dynamic_rulebook` folder, and you can run any of them by modifying and executing the `run_multi_dynamic.sh` script.
+
+```bash linenums="1"


I think quoting the entire script here is confusing. I'm not sure you need to quote any of it, but if you think that would be helpful, just quote the specific part people should modify.

dfremont · 2026-04-17T19:51:50Z


 *.cproject
+
+examples/dynamic_rulebook/*/outputs/


Let's move this to a new .gitignore file in the examples/dynamic_rulebook directory, so that it will keep working even if we move that folder later.

dfremont · 2026-04-17T20:00:03Z

+rm $scenario/outputs/$csv_file\_scatter.png
+if [ "$use_dynamic_rulebook" = true ]; then
+
+    for seed in $(seq 0 1);


Why are we running this twice with different fixed seeds?

dfremont · 2026-04-17T20:02:49Z

+    priority_graphs = {}
+    using_sampler = -1
+    verbosity = 1
+    exploration_ratio = 2.0
+    using_continuous = False


Surely all of these should be attributes of the rulebook object, not global state attached to the class? What if somebody wants to instantiate multiple rulebooks with different parameters?

dfremont · 2026-04-17T20:03:07Z

+    def visit_FunctionDef(self, node):
+        self.functions.append(node)
+
+class rulebook(ABC):


Suggested change

class rulebook(ABC):

class Rulebook(ABC):

dfremont · 2026-04-17T20:04:34Z

+                else:
+                    assert isinstance(subsampler, RandomSampler)
+        if not sorted(list(self.split_samplers.keys())) == list(range(len(rulebook.priority_graphs))):
+            raise ValueError('Priority graph IDs should be in order and start from 0')


Shouldn't this be an assertion? We're the ones deciding the structure of the rulebook object and how we're putting together the samplers here, not the user.

dfremont · 2026-04-17T20:06:18Z

+                                                                RandomSampler)
+            for subsampler in self.split_samplers[id].samplers:
+                if isinstance(subsampler, ContinuousDynamicCESampler):
+                    subsampler.set_graph(priority_graph)


Setting the graph in a second step seems unnecessary. Can't you just pass it in when creating the sampler originally? (in the lambdas above, which you'd have to move inside the loop)

dfremont · 2026-04-17T20:09:47Z

+        # Update each sampler based on the corresponding segment
+        try:
+            iter(rhos)
+        except:


Don't use bare except:, which will catch KeyboardInterrupt. If you really want to catch any ordinary exception you can use except Exception:, but in this case you seem to just be checking whether rho is iterable or not. Doesn't it have to be a tuple since we're doing multi-objective falsification?

dfremont · 2026-04-17T20:11:48Z

                    break
                if self.verbosity >= 2:
                    print("Sample no: ", i, "\nSample: ", sample, "\nRho: ", rho)
+                if self.dynamic and self.verbosity >= 1:


This should only happen at verbosity 2, in which case we just printed rho above, so why do we need this?

dfremont · 2026-04-17T20:13:54Z

+                for r in rho:
+                    self.populate_error_table(sample, r)


Why are we calling this multiple times? There should only be one row in the error table for each sample/simulation. The different elements of rho should become different columns, as in the old multi-objective falsification code, right?

OK, if I understand correctly, in the dynamic case you've changed rho to be a list of lists, one per segment. But it still doesn't make sense to have multiple rows for a single simulation, and the rho_0 column for example will mean different things in different rows, since the first rule varies depending on which segment was active. We'll need to have a discussion on Tuesday about what the error table should look like.

dfremont · 2026-04-17T20:18:18Z

+        if self.dynamic:
+            while result is None:
+                sample = self.get_sample(1)
+                scene = self.sampler.lastScene
+                assert scene
+                result = self._simulate(scene)


What is the purpose of this code? We should not be running more than one simulation per sample. If the simulation is rejected, the sampler should receive the rejection feedback as in the following lines.

dfremont · 2026-04-17T20:29:47Z

+            function_name = function_node.name
+            function_code = compile(ast.Module(body=[function_node], type_ignores=[]), '<string>', 'exec')
+            exec(function_code)
+            self.functions[function_name] = locals()[function_name]


Rather than all this rigamarole of compiling each function separately (which incidentally would break if the function refers to a constant defined at the top level of the module, for example), why not just exec the entire file into a new namespace and pull out all the functions? runpy.run_path might work for this too.

dfremont · 2026-04-17T21:05:08Z

+    def getVector(self):
+        return self.generateSample()
+
+    def generateSample(self):


What's the point of separating this method from getVector, which is the actual API required for a BoxSampler?

dfremont · 2026-04-17T21:07:51Z

+        except:
+            return
+        if len(rho) != self.num_properties:
+            return


Returning silently doesn't seem like the right thing to do. Shouldn't this be an assertion?

dfremont · 2026-04-17T21:10:16Z

+        is_ce = True
+        for node in self.priority_graph.nodes:
+            if rho[node] >= self.thres[node]:
+                is_ce = False
+                break


Is this the intended logic? It seems to say you only consider the sample to be a counterexample if it violates all of the specifications in the graph.

dfremont · 2026-04-17T21:13:36Z

+            row *= self.alpha
+            row[b] += 1 - self.alpha
+
+class DiscreteDynamicCESampler(DiscreteCrossEntropySampler):


Did you test this? I don't think it will work, since DiscreteCrossEntropySampler does not expect rho to be a list.

dfremont · 2026-04-17T21:16:20Z

                self.ignore_locs = self.ignore_locs + list(locs[0])
            sample_dict[k] = float(v) if self.column_type[k] and v is not None else v
-        if isinstance(rho, (list, tuple)):
+        if is_multi or isinstance(rho, (list, tuple)):


Why is this change needed? If rho is not a list/tuple, won't iterating over it fail? Or are you passing some new kind of iterable in for rho?

dfremont

Hi Kai-Chun, thanks for making the PR. I haven't finished reviewing the new samplers and the rulebook code itself, but won't get to it till later so I wanted to send what I have now. Besides my detailed comments above, some big-picture points:

There are important changes to the server and error table that don't seem necessary and which don't make sense to me. Please see my comments, and we can discuss in detail on Tuesday.
We need test cases for the new functionality. I'm pretty sure some of the code won't work (e.g. discrete domains with the new DCE sampler), which would come out if there were tests.
There is a lot of repeated code across the examples which I think could be substantially reduced, e.g. using decorators for rules that operate uniformly on trajectories (see our previous discussion), a helper function to segment trajectories based on when specified records become true, and a common base for the collect_result scripts. This isn't just to save lines: the code will be easier to understand if it has a simpler structure and the common parts are broken out into functions with informative names.
The description of the system in the README is great and very comprehensive, but I think it's long enough to be hard to navigate (it combines basic instructions on running the example with technical details about the APIs). Since we need to add a new page to the online documentation about this feature anyway, I would suggest moving the current README into the documentation (either as one big page with different sections [probably replacing the current Multi-Objective Falsification page], or possibly multiple pages) and having only the essentials in the README itself. You could include installation instructions, how to run the example, and a brief summary of what the different files are for, pointing to the documentation for details.

kevinchang73 added 9 commits December 1, 2025 16:45

Interface to Scenic 3; add examples from RV24

0d93359

Add examples for VerifAI 2.0

4923689

Remove debugging message

da70943

Refactor pipeline

f0f5368

Add README

8243d80

Update rulebook class

365d078

Remove udemab and emab samplers

6107c6e

Remove prints

e2d6cf9

Rename samplers

3384862

beyazit-y approved these changes Apr 9, 2026

View reviewed changes

dfremont reviewed Apr 17, 2026

View reviewed changes

dfremont requested changes Apr 17, 2026

View reviewed changes

	current_laneSection = network.laneSectionAt(self)
	current_laneSection = self.laneSection

		require (distance from blockingCar to intersection) > DIST_TO_INTERSECTION
		terminate when (distance to spawnPt) > TERM_DIST

		@@ -0,0 +1,19 @@
		import numpy as np

		def rule0(simulation, indices): # safe distance to obstacle

	python -m pip install -e .
	python -m pip install -e ".[examples]"


		We provide six different scenarios in the `examples/dynamic_rulebook` folder, and you can run any of them by modifying and executing the `run_multi_dynamic.sh` script.

		```bash linenums="1"


		*.cproject

		examples/dynamic_rulebook/*/outputs/

Conversation

kevinchang73 commented Dec 2, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dfremont Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

dfremont Apr 17, 2026 •

edited

Loading