Add SOG to waypoint legs and fix target ship position bug #83
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
tomarnepedersen
commented
Nov 26, 2025
tomarnepedersen
commented
- Added sog to each waypoint leg. Updated documentation. Generated new baseline_situations_generated files. Updated types.
- Fixed bug where future position of target ship was used instead of initial position of target ship to check encounter evolvement. Updated documentation to make this functionality clearer.
- Added option to pass in a single input situation .json file, not only a folder.
…functionality with folder)
…ial ownship position and location of encounter
Add cli options: added option to read in a single situation file, added option to set ownship coordinate lat,lon as commandline argument, updated/extended README with testing instructions, added section to README on Contributing, fixed pydantic warning on definition of path_type, and added new tests for above functionality (single situation file, lat-lon argument).
…et ship instead of future position.
…e-encounter-has-evolved-initial-position-of-the-target-ship-should-be-used-instead-of-the-future-position Resolves #77, fixed bug, encounter evolvement is using initial position of target ship.
…-traffic-situation
…points-in-generated-traffic-situation Resolves #78 - Adding sog to waypoint legs for all vessels.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds SOG (Speed Over Ground) to waypoint legs, fixes a critical bug in encounter evolution checking, and enables passing a single .json file as input instead of requiring a folder.
Key changes:
- Added
sogfield to waypointLegtype and populated it throughout the codebase - Fixed bug where future target ship position was incorrectly used instead of initial position for encounter evolution validation
- Added support for reading single .json situation files (not just folders)
Reviewed changes
Copilot reviewed 72 out of 74 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/trafficgen/types.py |
Added sog field to Leg class; modified path_type default behavior |
src/trafficgen/encounter.py |
Fixed target ship position bug (line 190); added SOG to waypoint legs; added ownship coordinate override logic |
src/trafficgen/read_files.py |
Refactored to support single .json file input; extracted file reading logic |
src/trafficgen/ship_traffic_generator.py |
Added ownship coordinate CLI parameter with validation |
src/trafficgen/write_traffic_situation_to_file.py |
Added unit conversion for leg SOG from m/s to knots |
src/trafficgen/cli.py |
Added --ownship-coordinate option; improved help text |
tests/test_trafficgen.py |
Added test for ownship coordinate override feature |
tests/test_read_files.py |
Added test for single file reading |
tests/conftest.py |
Added situations_file fixture; fixed docstring typo |
docs/source/*.rst |
Clarified documentation for encounter evolution and added SOG examples |
pyproject.toml, CITATION.cff |
Version bump to 0.8.4; added Stephanie Kemna to contributors |
CHANGELOG.md |
Documented changes in version 0.8.4 |
README.md |
Added comprehensive contributing guidelines |
data/baseline_situations_generated/*.json |
Regenerated 55 baseline files with version 0.8.3→0.8.4 and added SOG to legs |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| testers = [ | ||
| { name = "Grunde Løvoll", email = "Grunde.Lovoll@dnv.com" }, | ||
| { name = "Stephanie Kemna", email = "Stephanie.Kemna@dnv.com" }, | ||
| { name = "Stephanie Kemna"}, |
There was a problem hiding this comment.
Missing email address for Stephanie Kemna in the testers list, while it was present in the removed line. This creates an inconsistency in the contributor information.
Comment on lines
233
to
238
| Field( | ||
| default=PathType.RTZ, | ||
| description=( | ||
| "Specifies the control-point model (e.g., Bezier, RTZ) " | ||
| "used to define the path for the ship to follow." | ||
| ), | ||
| ), |
There was a problem hiding this comment.
The field path_type is missing from the Field() constructor where the default was previously set with default=PathType.RTZ. Moving the default to the field definition without keeping it in the Field() can change validation behavior. The removed line should be restored: default=PathType.RTZ,
| { name = "Tom Arne Pedersen", email = "Tom.Arne.Pedersen@dnv.com" }, | ||
| { name = "Claas Rostock", email = "Claas.Rostock@dnv.com" }, | ||
| { name = "Minos Hemrich", email = "Minos.Hemrich@dnv.com" }, | ||
| { name = "Stephanie Kemna"} |
There was a problem hiding this comment.
Missing email address for Stephanie Kemna in the authors list. The other authors have email addresses specified, so this should be consistent.
Collaborator
Author
|
Review allready done on dev branch |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.