Skip to content

Fix seven defects found while documenting 0.5.1 - #15

Merged
schreck61 merged 8 commits into
mainfrom
fix-0.5.1-defects
Nov 11, 2026
Merged

schreck61 merged 8 commits into
mainfrom
fix-0.5.1-defects

Conversation

@schreck61

Copy link
Copy Markdown
Owner

Seven defects turned up while docs/architecture.md was checked claim by claim against the 0.5.1 code. Each was confirmed in the code, and where it shows, in the app, before it was changed. Each fix is a commit of its own, with its test, so they can be reviewed one at a time. The last commit updates the architecture document.

Defect Fix Test
Hybrid weights files that don't load. Save weights passed NaN as the score, and train saved an infinite one when no static tuning passed the pedestrian check. JSON writes both as null, which SavedHybrid rejected. weights::load returned the first readable copy without checking that it loads, so such a file also hid the other copies. base_score is an Option<f64>: a score that isn't finite is saved as unknown, and 0.5.1's null loads. weights::load takes the parser and skips a copy that doesn't load. The app saves the network only when it is trained or learning (has_model || learning), not an untrained one. tests/weights_files.rs: saving with each kind of score, with and without a network, then loading; a 0.5.1 file with null; a damaged copy skipped for the next one and for the built-in one
The green wave lost the controllers. It switched every signal to fixed-time, and turning it off left them that way. World remembers each signal's controller when the wave starts and gives it back when the wave ends, unless another was chosen meanwhile through the new World::set_controller. Rebuilds during the wave keep the remembered controller. The tooltip says what the wave does, and the bottom panel confirms each change. New tests/green_wave.rs: a four-signal corridor with mixed controllers, one chosen during the wave, run without collisions; a single intersection left alone
The roundabout's pedestrian tooltip said pedestrians use underground passages. Each layout without crosswalks gets its own reason. UI text only
A Hybrid whose saved network doesn't fit said "no network beat the best static tuning". Hybrid records that the network was left out (model_rejected), and the brain says "the saved network doesn't fit this layout". tests/weights_files.rs: both messages
The signal timing chart never showed yellow or all-red. Each span ran from one green to the next. StageSpan has a phase; the history records the yellow and all-red as their own spans (1,200 kept, up from 400), and the chart draws them in yellow and red. Unit test in sim/signal.rs
The margin over the gap showed a negative value as "+0.00 s", in red. margin_kpi keeps the sign. Rounding error just below zero still reads "+0.00 s". Unit test in gui/mod.rs
In a corridor, the Controller list and status overlay showed the last choice, not the focused intersection's controller, after the focus moved or the green wave changed controllers. App::sync_ctrl reads both sides' controllers from the focused intersection after a rebuild and at the end of each frame, and asks for another frame when they changed. None: App::new needs an eframe creation context. Checked in the app

Checks

  • cargo build --locked --release --all-targets
  • cargo clippy --locked --release --all-targets -- -D warnings
  • cargo clippy --locked --release --target wasm32-unknown-unknown --bin intersection-lab -- -D warnings
  • cargo test --locked --release
  • cargo deny --locked check
  • The weights fuzz target for 60 s: 271,436 runs, no crashes. That run came before rebasing onto the current main, which didn't touch the weights code.

Every commit builds and passes clippy on its own.

In the app, with the scripted-screenshot settings: the green wave on and off on a corridor with mixed controllers, the timing chart at 120 s, and a three-signal corridor with different controllers as the focus moves.

Not tested in the app:

  • The roundabout's tooltip: hovering can't be scripted.
  • A negative margin over the gap: the cooperative tests require every margin to be at least −10⁻⁶ s, so the app doesn't produce one. The unit test covers the formatting.

Traffic behavior

None of these change how traffic runs. Only the timing chart reads the signal history, and a weights copy that loads is used as before, so bench results are unchanged.

Documentation

docs/architecture.md now says it describes the code on main: 0.5.1 and the fixes since. It covers the loading order and outcomes in §8.7, the green wave in §6.10 and the tour, the signal history and timing chart, the margin's sign, the tests, the file sizes and the repository layout. timeline.png was captured again with the Appendix A settings; only the yellow and red segments changed. The whole-window images, such as overview with its hand-drawn callouts, still show the 0.5.1 chart at small scale.

A possible follow-up

The green wave switches both sides of a comparison to fixed-time, so side B can't run an adaptive controller against a green wave on side A. Applying the wave only to signals already running fixed-time would allow that comparison. That changes how the feature works, so it isn't part of this PR.

The app's Save weights passed NaN as the Hybrid's score, and train saved
an infinite score when no static tuning passed the pedestrian check.
JSON stores both as null, which SavedHybrid couldn't read, so the file
didn't load. And because weights::load returned the first readable copy
without checking that it loads, such a file also hid the user's other
copy and the built-in one: the Hybrid fell back to the planner's default
tuning.

* base_score is now an Option<f64>. A score that isn't finite is saved
  as unknown, and the null that 0.5.1 wrote loads.
* weights::load takes the parser and returns the first copy it accepts,
  so a copy that doesn't load is skipped for the next one.
* The app saves the Hybrid's network only when it is trained or
  learning. It used to save an untrained network too, which a Hybrid
  loading the file would then follow, now that the file loads.

tests/weights_files.rs saves with each kind of score, with and without
a network, and loads the file back; loads a 0.5.1 file with a null
score; and checks that a damaged copy is skipped for the next one and,
failing that, for the built-in copy.

Signed-off-by: Jim Schreckengast <schreck@gmail.com>
A Hybrid weights file whose network doesn't fit the layout loads with
its base tuning only, and the controller brain then said "no network
beat the best static tuning", which is what a file saved without a
network means. Hybrid now records that the network was left out
(model_rejected), and the brain says "the saved network doesn't fit
this layout" instead.

tests/weights_files.rs checks both messages.

Signed-off-by: Jim Schreckengast <schreck@gmail.com>
The signal's history recorded one span per stage, from the start of its
green to the start of the next green, so the bottom panel's timeline
drew the yellow change and the all-red clearance as part of the green.

Each StageSpan now has a phase, and the history records the yellow and
the all-red that end a green as spans of their own. The timeline draws
them in yellow and red after the green. The history keeps 1,200 spans,
up from 400, since a stage change can now add three.

A unit test in sim/signal.rs checks that a change records green,
yellow, all-red and the next green, with the right durations and no
gaps.

Signed-off-by: Jim Schreckengast <schreck@gmail.com>
The safety monitors showed the smallest margin over the required gap
clamped at zero, so a crossing that broke its gap read "+0.00 s", in
red. margin_kpi now formats the real value with its sign, so such a
margin reads, for example, "-0.25 s". Rounding error just below zero
still reads "+0.00 s", in green.

A unit test in gui/mod.rs covers a positive, a negative, a rounding
error and an unset margin.

Signed-off-by: Jim Schreckengast <schreck@gmail.com>
The Pedestrian spawn tool is disabled on the layouts without crosswalks,
and its tooltip said "pedestrians use underground passages" on all of
them. That is true of the cooperative crossing only; the roundabout is
simulated without pedestrians. Each layout now gets its own reason.

Signed-off-by: Jim Schreckengast <schreck@gmail.com>
In a corridor, the Controller list and the status overlay showed the
last controller chosen, not the one the focused intersection runs, once
the focus moved to another intersection. They also kept showing the old
controller after the green wave switched every signal to fixed-time.

App::sync_ctrl now reads both sides' controllers from the focused
intersection after a rebuild and at the end of every frame, and asks
for another frame when they changed, since panels drawn earlier in the
frame showed the old ones. A rebuild such as Reset uses the controller
the list shows.

App::new needs an eframe creation context, so this has no automated
test. It was checked in the app with scripted screenshots of a
three-signal corridor with different controllers, with the focus moved.

Signed-off-by: Jim Schreckengast <schreck@gmail.com>
The corridor's green wave switches every signal to fixed-time. Turning
it off left them all fixed-time, so the controllers they ran before
were lost.

World now remembers each signal's controller when the wave starts
(controller_before_wave) and gives it back, when the wave ends, to each
signal still running fixed-time. A controller chosen during the wave,
through the new World::set_controller, is kept instead. A rebuild during
the wave, such as Reset, builds with the remembered controller, so
ending the wave still restores it. The green wave's tooltip now says
what it does to the controllers, and the bottom panel confirms each
change.

tests/green_wave.rs turns the wave on and off on a four-signal corridor
with mixed controllers, one of them chosen during the wave, runs it
without collisions, and checks that a single intersection is left
alone.

Signed-off-by: Jim Schreckengast <schreck@gmail.com>
* Section 8.7: each copy of a weights file is tried until one loads,
  Hybrid files with a score that isn't finite load, and a Hybrid whose
  network doesn't fit says so. The flowchart and the table of outcomes
  follow.
* Section 6.10 and the tour: the green wave gives back the controllers
  it replaced, and the Controller list and status overlay follow the
  focused intersection.
* The signal history and the timing chart: the yellow and all-red are
  recorded and drawn. timeline.png is captured again with the settings
  in Appendix A; only the yellow and red segments changed.
* The margin over the gap shows its sign.
* Tests, file sizes and the repository layout include the new tests.

The document now says it describes the code on main: 0.5.1 and the
fixes since.

Signed-off-by: Jim Schreckengast <schreck@gmail.com>
@schreck61
schreck61 enabled auto-merge (rebase) November 11, 2026 13:32
@schreck61
schreck61 merged commit 3a8397f into main Nov 11, 2026
12 checks passed
@schreck61
schreck61 deleted the fix-0.5.1-defects branch November 11, 2026 15:48
@schreck61 schreck61 mentioned this pull request Nov 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant