Skip to content

Commit 9a7ce9e

Browse files
bedaHovorkaclaude
andcommitted
sim(#750): finalize waitCrossing conversion — fix tolerance gap, correct comment, add regression tests
Verifies and closes out the waitCrossing conversion from 16fb70c now that kDisco 0.6.1-SNAPSHOT (with waitCrossing) is locally publishable and the build works again: - Generator.kt: correct a comment that overclaimed velocity-target events are root-found — only block-boundary and tail-entry were converted; Motor's AccelerationStopCondition still uses plain waitUntil. The Motor/dtMax follow-up is now tracked separately in issue #760. - SimpleTestProcess.kt: fix a real regression exposed by the conversion. This test coordinator never runs Generator, so it silently relied on kDisco's own raw defaults (dtMin=1e-5, maxAbsError=1e-5 — nearly equal in magnitude). The waitCrossing guards intentionally leave a structural gap of exactly `dtMin` at the moment a threshold is reached (see the existing Train.kt comments on why that slack is necessary), which is negligible against the intended maxAbsError=1e-2 but trips LengthChecker's invariant when left at kDisco's near-equal raw defaults. Confirmed via A/B test against 74cd41b (same kDisco version, pre-conversion Train.kt) that this is new, not pre-existing. Fixed by configuring the same tolerances Generator.startAction() sets. - SimpleTestProcessTest.kt: relax an assertion that assumed `position` stays >= 0 immediately after a block-boundary crossing. The waitCrossing guard deliberately fires `dtMin` short of the boundary so it reliably crosses zero instead of asymptoting forever; this leaves a bounded, harmless negative transient that was never a documented invariant. - SimpleLinearTrackTestProcessTest.kt: add an explicit regression test proving both converted call sites fire correctly end-to-end. Verified locally: kDisco published to mavenLocal from copilot/add-zero-crossing-detection (feaa058), full build/test/ integrationTest/heavyTest all green (0 failures across all JUnit reports). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 16fb70c commit 9a7ce9e

4 files changed

Lines changed: 81 additions & 7 deletions

File tree

core/src/commonMain/kotlin/cz/vutbr/fit/interlockSim/sim/Generator.kt

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,13 @@ open class Generator(
3131

3232
override suspend fun startAction() {
3333
dtMin = 1e-6
34-
// Block-boundary and velocity-target events are now located by kDisco root-finding
34+
// Block-boundary and tail-entry events are now located by kDisco root-finding
3535
// (`Process.waitCrossing`, see Train.kt) rather than by step granularity, so `dtMax` no
36-
// longer has to be tiny to keep event overshoot negligible. Raising it to kDisco's natural
37-
// value lets the adaptive RKF45 error controller pick large steps during cruise, cutting
38-
// derivative evaluations by orders of magnitude (Issue #750).
36+
// longer has to be tiny to keep *those* events' overshoot negligible. Motor's
37+
// AccelerationStopCondition (velocity-target detection, Train.kt) is NOT yet converted —
38+
// it still relies on step granularity via plain `waitUntil` — so `dtMax` is deliberately
39+
// left unchanged here rather than raised further; converting Motor and re-tuning dtMax
40+
// is tracked separately (Issue #760, follow-up to #750).
3941
dtMax = 1e-3
4042
maxRelError = 1e-2
4143
maxAbsError = 1e-2

core/src/commonTest/kotlin/cz/vutbr/fit/interlockSim/sim/SimpleTestProcess.kt

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@
1010
package cz.vutbr.fit.interlockSim.sim
1111

1212
import cz.ksimulantenbande.kdisco.Process
13+
import cz.ksimulantenbande.kdisco.dtMax
14+
import cz.ksimulantenbande.kdisco.dtMin
15+
import cz.ksimulantenbande.kdisco.maxAbsError
16+
import cz.ksimulantenbande.kdisco.maxRelError
1317
import io.github.oshai.kotlinlogging.KotlinLogging
1418

1519
/**
@@ -121,6 +125,18 @@ class SimpleTestProcess(
121125
* The train's actions() method will then begin executing.
122126
*/
123127
override suspend fun startAction() {
128+
// Mirrors Generator.startAction()'s physics tolerances (see Generator.kt). Unlike
129+
// Generator, this coordinator never runs one, so without this the simulation falls
130+
// back to kDisco's own raw defaults (dtMin=1e-5, maxAbsError=1e-5 -- nearly equal in
131+
// magnitude). Train's block-boundary/tail-entry waitCrossing guards (Train.kt) are
132+
// designed to leave a structural gap of exactly `dtMin` at the moment a threshold is
133+
// reached (see Train.kt comments), which is negligible against the intended
134+
// maxAbsError=1e-2 but can spuriously trip LengthChecker's invariant when dtMin and
135+
// maxAbsError are left at kDisco's near-equal raw defaults instead.
136+
dtMin = 1e-6
137+
dtMax = 1e-3
138+
maxRelError = 1e-2
139+
maxAbsError = 1e-2
124140
logger.debug { "SimpleTestProcess: Starting test with train ${train.name}" }
125141
Process.activate(train) // CRITICAL: activate the train process
126142
}

core/src/jvmTest/kotlin/cz/vutbr/fit/interlockSim/sim/SimpleLinearTrackTestProcessTest.kt

Lines changed: 52 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,14 +61,15 @@ class SimpleLinearTrackTestProcessTest : KoinTestBase() {
6161

6262
private fun specAB(
6363
inTime: Double = 1.0,
64-
outTime: Double = 15.0
64+
outTime: Double = 15.0,
65+
length: Double = 20.0
6566
): SimpleLinearTrackTestProcess.TrainSpec =
6667
SimpleLinearTrackTestProcess.TrainSpec(
6768
inName = "A",
6869
outName = "B",
6970
inTime = inTime,
7071
outTime = outTime,
71-
length = 20.0
72+
length = length
7273
)
7374

7475
/** Scenario 1: train follows a pre-reserved path A→B and makes genuine forward progress. */
@@ -244,4 +245,53 @@ class SimpleLinearTrackTestProcessTest : KoinTestBase() {
244245
assertThat(train.totalDistance).isGreaterThan(distanceBeforeRelease)
245246
assertThat(process.getBlockTransitions(train.name)).isGreaterThan(0)
246247
}
248+
249+
/**
250+
* Scenario 5 (Issue #750/#760 follow-up): proves the `waitCrossing` block-boundary
251+
* conversion (Train.kt:254) actually locates each section boundary rather than hanging.
252+
* The A->Sem->B topology (linearPathWithSemaphoreSimulation) has two 100.0m blocks, so
253+
* reaching B means both boundary crossings fired; total distance exceeding the first
254+
* block's length is direct evidence the A->Sem crossing was located correctly.
255+
*/
256+
@Test
257+
@Timeout(value = 60, unit = TimeUnit.SECONDS)
258+
fun `train crosses both block boundaries and reaches destination`() {
259+
val ctx = loadLinearContext()
260+
val inOuts = ctx.getInOuts().toList()
261+
val a = inOuts.single { it.name == "A" }
262+
val b = inOuts.single { it.name == "B" }
263+
val reservationService = ctx.getRoutingServices().getPathReservationService()
264+
265+
var capturedTrain: Train? = null
266+
val process =
267+
SimpleLinearTrackTestProcess(
268+
ctx,
269+
endTime = 50L,
270+
trainSpecs = listOf(specAB()),
271+
onTrainCreated = { train ->
272+
capturedTrain = train
273+
val res = reservationService.reservePath(train.name, a, b)
274+
assertThat(res).isInstanceOf<PathReservationService.ReservationResult.Success>()
275+
}
276+
)
277+
ctx.setMainProcess(process)
278+
ctx.run()
279+
280+
val train = requireNotNull(capturedTrain) { "onTrainCreated was never called" }
281+
// Reaching B proves both waitCrossing block-boundary calls (A->Sem, Sem->B) fired.
282+
assertThat(process.getTrainsExited()).isEqualTo(1)
283+
// Exceeding the first block's length (100.0m) is direct evidence the A->Sem
284+
// waitCrossing call actually located that boundary rather than stalling on it.
285+
assertThat(train.totalDistance).isGreaterThan(100.0)
286+
}
287+
288+
// Note: the tail-entry "already satisfied" branch (Train.kt:1072, `if (front.getTotalDistance()
289+
// < tailEntryThreshold)`) is only reachable when `length <= dtMin` (1e-6) -- but `dtMin` is
290+
// itself far below `ContinuousInvariantChecker`'s `maxAbsError` (1e-2), i.e. below the
291+
// simulation's own numerical noise floor for front/tail position tracking. An end-to-end
292+
// scenario exercising that branch (tried during this change's verification, e.g.
293+
// length = 1e-7) reliably trips the invariant checker's FATAL guard well before completion --
294+
// independent of waitUntil vs waitCrossing, since it is a property of train-length realism,
295+
// not of the crossing-detection primitive. This degenerate branch is effectively unreachable
296+
// for any physically valid train and is not exercised here for that reason.
247297
}

core/src/jvmTest/kotlin/cz/vutbr/fit/interlockSim/sim/SimpleTestProcessTest.kt

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,13 @@ class SimpleTestProcessTest : KoinTestBase() {
247247
val finalState = testProcess.getTrainState()
248248
assertThat(finalState).isNotNull()
249249
assertThat(finalState.velocity).isGreaterThanOrEqualTo(0.0)
250-
assertThat(finalState.position).isGreaterThanOrEqualTo(0.0)
250+
// `position` can be transiently negative by up to `dtMin` immediately after a
251+
// block-boundary crossing (Train.kt:257, `position.state -= nextLength`) -- the
252+
// `waitCrossing` guard intentionally fires `dtMin` short of the boundary so it
253+
// reliably crosses zero rather than asymptoting forever (see Train.kt comments
254+
// on the block-boundary waitCrossing call). This was never a documented
255+
// invariant; tolerate the bounded transient rather than requiring an exact >= 0.
256+
assertThat(finalState.position).isGreaterThanOrEqualTo(-1e-5)
251257
}
252258
}
253259
}

0 commit comments

Comments
 (0)