Skip to content

Commit 6bbbff5

Browse files
CopilotMathiasVDA
andauthored
docs: address PR review feedback — consistency, grammar, and architecture alignment (#2)
* Initial plan * docs: apply PR review feedback - fix grammar, spelling, defaults, and architecture docs Co-authored-by: MathiasVDA <15101339+MathiasVDA@users.noreply.github.com> Agent-Logs-Url: https://github.com/Matdata-eu/tp-lib/sessions/02a3fc18-73f4-4e65-93fe-e859f0d191c0 --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: MathiasVDA <15101339+MathiasVDA@users.noreply.github.com>
1 parent 1fd9b2d commit 6bbbff5

File tree

9 files changed

+29
-24
lines changed

9 files changed

+29
-24
lines changed

.github/workflows/coverage.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ jobs:
3131
3232
- name: Upload coverage to Codecov
3333
uses: codecov/codecov-action@v4
34+
if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository
3435
with:
3536
files: lcov.info
3637
token: ${{ secrets.CODECOV_TOKEN }}

CONTRIBUTING.md

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -292,9 +292,8 @@ When working on path calculation features (`tp-core/src/path/`):
292292
The path calculation module consists of several submodules:
293293

294294
- **`candidate.rs`**: Find candidate netelements for each GNSS position
295-
- **`probability.rs`**: Calculate probabilities using distance and heading
296-
- **`construction.rs`**: Build paths forward and backward through network
297-
- **`selection.rs`**: Select best path from candidates
295+
- **`probability.rs`**: Calculate HMM-related probabilities (e.g., emission/transition) using distance, heading, and network context
296+
- **`viterbi.rs`**: Run the HMM/Viterbi algorithm to compute the most likely train path from the candidate sequences
298297
- **`graph.rs`**: Network topology graph operations
299298
- **`spacing.rs`**: GNSS resampling for consistent spacing
300299

README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99

1010
**Status**: Under construction
1111

12-
Train positioning library excels in post-processing the GNSS positions of your measurement train to achieve an unambiguous network location. This library is your a map matching assistant specifically for railway.
12+
Train positioning library excels in post-processing the GNSS positions of your measurement train to achieve an unambiguous network location. This library is your map matching assistant specifically for railway.
1313

1414
## Features
1515

@@ -52,7 +52,7 @@ tp-cli --gnss positions.csv --network network.geojson --train-path path.csv --ou
5252
### Debug Output
5353

5454
Pass `--debug` to write intermediate HMM calculation results as GeoJSON files to a `debug/` subdirectory next to the output file.
55-
See **[DEBUG.md](DEBUG.md)** for a full description of the four output files, their properties, and a typical debugging workflow.
55+
See **[DEBUG.md](DEBUG.md)** for a full description of the debug output files, their properties, and a typical debugging workflow.
5656

5757
### Algorithm Parameters
5858

specs/002-train-path-calculation/algorithm.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -242,9 +242,9 @@ When **all** transition scores at a time-step `t` are `-∞` (no feasible transi
242242
3. For each current candidate `j` with non-zero emission: `log_V[t][j] = carry_score + ln(P_emission(t, j))`
243243
4. Set `backptr[t][j] = i*` so the backtrace follows the best previous state
244244

245-
This produces a **single unbroken subsequence** for all GNSS input (the GNSS data represents one continuous drive). The heavy penalty ensures that carry-forward transitions are strongly disfavoured relative to genuine topological transitions, but the chain is never severed.
245+
This produces a **single unbroken subsequence** within a Viterbi processing window. The heavy penalty ensures that carry-forward transitions are strongly disfavoured relative to genuine topological transitions, but within a window the chain is never severed.
246246

247-
**Important**: Because carry-forward preserves chain continuity, the backtrace always yields exactly one subsequence covering the entire GNSS timeline.
247+
**Important**: Because carry-forward preserves chain continuity *within a window*, the backtrace for that window always yields exactly one subsequence covering the entire GNSS timeline of the window. Requirement **FR-027** (Viterbi break detection and subsequence reinitialization) is satisfied by higher-level control logic, which may terminate the current window and start a new one when configured break conditions are met; in that case, multiple subsequences exist across windows, while each individual window still uses the no-break penalty carry-forward scheme described here.
248248

249249
### Backtrace
250250

specs/002-train-path-calculation/contracts/cli.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,12 @@ This is the **primary workflow** for path-based GNSS projection. It combines pat
147147
| `--format <FORMAT>` | auto | Output format: `csv`, `geojson`, or `auto` (detect from extension) |
148148
| `--save-path <FILE>` | None | Optionally save calculated path to file (in addition to projected coordinates) |
149149

150+
**Input Options:**
151+
152+
| Option | Default | Description |
153+
|--------|---------|-------------|
154+
| `--crs <CRS>` | None | Coordinate Reference System for GNSS input (e.g., `EPSG:4326`). Overrides CRS column in CSV file. |
155+
150156
**General Options:**
151157

152158
| Option | Description |

specs/002-train-path-calculation/contracts/lib-api.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ pub struct PathConfig {
4646
/// Heading scale for exponential decay (default: 2.0 degrees)
4747
pub heading_scale: f64,
4848

49-
/// Maximum distance for candidate selection (default: 50.0 meters)
49+
/// Maximum distance for candidate selection (default: 500.0 meters)
5050
pub cutoff_distance: f64,
5151

5252
/// Maximum heading difference before rejection (default: 10.0 degrees)

specs/002-train-path-calculation/spec.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ A developer troubleshooting path calculation issues exports intermediate results
152152

153153
### Session 2026-01-08
154154

155-
- Q: When GNSS coordinates fall outside the configured cutoff distance (default 50m) from all track segments during the projection phase (after path is calculated), how should the system handle these outliers? → A: Exclude outlier coordinates from output entirely (omit from results file). Future feature will address better handling.
155+
- Q: When GNSS coordinates fall outside the configured cutoff distance (default 500m) from all track segments during the projection phase (after path is calculated), how should the system handle these outliers? → A: Exclude outlier coordinates from output entirely (omit from results file). Future feature will address better handling.
156156
- Q: How should the distance between a GNSS coordinate and a candidate netelement be factored into the probability calculation? → A: Inverse exponential decay based on both distance (e.g., e^(-distance/scale)) and heading difference (e.g., e^(-heading_diff/scale))
157157
- Q: When multiple candidate paths have identical probability scores (after forward/backward averaging), which path should be selected? → A: Select the first path found during calculation (arbitrary but deterministic)
158158
- Q: When a pre-calculated train path is provided as input (FR-041), what format should the system expect? → A: Same format as path-only export: CSV or GeoJSON with ordered AssociatedNetElements
@@ -173,7 +173,7 @@ A developer troubleshooting path calculation issues exports intermediate results
173173

174174
### Edge Cases
175175

176-
- GNSS coordinates more than the configured cutoff distance (default 50m) from any track segment are excluded from output (omitted from results)
176+
- GNSS coordinates more than the configured cutoff distance (default 500m) from any track segment are excluded from output (omitted from results)
177177
- NetRelations where elementA equals elementB (self-referencing) are skipped with warnings logged
178178
- NetRelations referencing non-existent netelement IDs are skipped with warnings logged; segments with only invalid netrelations are treated as isolated
179179
- What happens when a track segment has no netrelations connecting it to other segments (isolated segment)?
@@ -216,7 +216,7 @@ A developer troubleshooting path calculation issues exports intermediate results
216216
- **FR-015**: The calculated train path MUST be continuous (each segment connects to the next via a netrelation)
217217
- **FR-016**: All netrelations between consecutive segments in the path MUST have navigability in the direction of travel (not "none" or opposing direction)
218218
- **FR-017**: System MUST find at most N nearest netelements for each GNSS coordinate (where N is configurable, default 3)
219-
- **FR-018**: System MUST only consider netelements within a configurable cutoff distance (default 50 meters) from each GNSS coordinate
219+
- **FR-018**: System MUST only consider netelements within a configurable cutoff distance (default 500 meters) from each GNSS coordinate
220220
- **FR-018a**: System MUST exclude from output any GNSS coordinates that are more than the cutoff distance from all track segments in the calculated path
221221
- **FR-019**: System MUST calculate probability for each candidate netelement using inverse exponential decay for both distance (e.g., e^(-distance/distance_scale)) and heading alignment (e.g., e^(-heading_difference/heading_scale)), with the overall probability being the product of distance and heading probability factors
222222
- **FR-020**: System MUST set probability to 0 when heading difference between GNSS coordinate and netelement exceeds configurable cutoff (default 10 degrees), overriding exponential decay calculation
@@ -324,7 +324,7 @@ A developer troubleshooting path calculation issues exports intermediate results
324324
The following configuration parameters are referenced in the requirements with default values:
325325

326326
- **Max nearest netelements**: Default 3 — maximum number of candidate track segments considered for each GNSS coordinate
327-
- **Distance cutoff**: Default 50 meters — maximum distance from GNSS coordinate to consider a track segment as candidate
327+
- **Distance cutoff**: Default 500 meters — maximum distance from GNSS coordinate to consider a track segment as candidate
328328
- **Heading difference cutoff**: Default 10 degrees — maximum heading misalignment before emission probability is set to 0
329329
- **Minimum probability threshold**: Default 2% — minimum emission probability for segment inclusion
330330
- **Resampling distance**: Default 10 meters — target spacing between GNSS coordinates used for path calculation

specs/002-train-path-calculation/tasks.md

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -133,18 +133,17 @@
133133
- [X] T063 [US1] Write unit test for consecutive position identification in tests/unit/path_probability_test.rs
134134
- [X] T064 [US1] Write unit test for coverage factor calculation in tests/unit/path_probability_test.rs
135135

136-
#### Phase 4: Path Construction (Bidirectional)
136+
#### Phase 4: Path Decoding (HMM / Viterbi)
137137

138138
- [X] T065 [P] [US1] Create tp-core/src/path/construction.rs module file
139-
- [X] T066 [P] [US1] Implement construct_forward_path() starting from highest probability netelement at first position
140-
- [X] T067 [P] [US1] Implement construct_backward_path() starting from highest probability netelement at last position
141-
- [X] T068 [US1] Implement graph traversal with navigability constraints using petgraph neighbors()
142-
- [X] T069 [US1] Implement probability threshold filtering (default 25%, except when only navigable option)
143-
- [X] T070 [US1] Implement path reversal for backward path (reverse segment order + swap intrinsic coordinates)
144-
- [X] T071 [US1] Implement bidirectional validation comparing forward and reversed backward paths
145-
- [X] T072 [US1] Write unit test for forward path construction in tests/unit/path_construction_test.rs
146-
- [X] T073 [US1] Write unit test for backward path construction and reversal in tests/unit/path_construction_test.rs
147-
- [X] T074 [US1] Write unit test for bidirectional agreement detection in tests/unit/path_construction_test.rs
139+
- [ ] T066 [P] [US1] Implement candidate_netelements_for_positions() to select candidate netelements per GNSS position (top-N by probability with navigability constraints)
140+
- [ ] T067 [P] [US1] Implement emission_probability() functions using per-position/per-netelements probability components (position, heading, coverage)
141+
- [ ] T068 [US1] Implement transition_probability() modeling between consecutive candidate netelements using topology connectivity and direction of travel
142+
- [ ] T069 [US1] Implement viterbi_decode_path() HMM decoder over the candidate lattice to select the most probable netelement sequence
143+
- [ ] T070 [US1] Implement insert_bridge_segments() to add required connecting segments between chosen netelements based on network topology
144+
- [ ] T071 [US1] Implement calculate_train_path() orchestrator calling candidate selection, emission/transition probability, Viterbi decoding, and bridge insertion
145+
- [ ] T072 [US1] Write unit tests for viterbi_decode_path() in tests/unit/path_construction_test.rs
146+
- [ ] T073 [US1] Write unit tests for insert_bridge_segments() and calculate_train_path() in tests/unit/path_construction_test.rs
148147

149148
#### Phase 5: Path Selection
150149

test-data/README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -543,7 +543,7 @@ Good result:
543543
target/release/tp-cli.exe --gnss test-data/log_29584/log_29584_L36-A_to_L36C-A_to_L25N-B.csv --crs EPSG:4326 --network test-data/network_airport.geojson --output test-data/log_29584/log_29584_L36-A_to_L36C-A_to_L25N-B-path-projection.geojson
544544
```
545545

546-
Good result (and proves the need to have longitudal redistribution of the gnss positions):
546+
Good result (and proves the need to have longitudinal redistribution of the gnss positions):
547547

548548
![L36-A to L36C-A to L25N-B (log_29584) - Path projection](log_29584/log_29584_L36-A_to_L36C-A_to_L25N-B-path-projection.png)
549549

@@ -640,7 +640,7 @@ Zoom on detail:
640640
target/release/tp-cli.exe --gnss test-data/log_28586/log_28586_L36-A_to_L36C-A_to_L25N-B-very-bad.csv --crs EPSG:4326 --network test-data/network_airport.geojson --output test-data/log_28586/log_28586_L36-A_to_L36C-A_to_L25N-B-very-bad-path-projection.geojson
641641
```
642642

643-
Expected result, again showing the need to also perform longitudal post processing:
643+
Expected result, again showing the need to also perform longitudinal post processing:
644644

645645
![L36-A to L36C-A to L25N-B very bad GNSS - Path projection](log_28586/log_28586_L36-A_to_L36C-A_to_L25N-B-path-projection.png)
646646

0 commit comments

Comments
 (0)