rodrigoqueiroz / geoscenarioserver

8 stars 1 forks source link

Combine scenarios at runtime #62

Closed scott-larter closed 3 years ago

scott-larter commented 3 years ago

Pull request to integrate loading and merging multiple scenario files at runtime.

Can now specify multiple scenario files with the -s flag. The first file listed will be the base scenario and its origin and globalconfig will be used. Any files after that will have their agents and elements merged into the base scenario to create a combined scenario.

Requirements for testing:

Note: this functionality is currently only implemented for loading all scenarios from .osm files and not for loading scenarios from code.

scott-larter commented 3 years ago

Just a few questions:

1. if every file contains the `globalconfig`, only the one from the first file is used and the other ones are ignored?

* yes, I see that you have the `UNIQUE_GS_TAGS_PER_SCENARIO` to handle that. Great!

1. I feel that the duplicate IDs problem should be automatically solved by appending something to disambiguate. For example, say we have a vehicle with ID=1 in both .osm files. When we read the first file, we see that the ID value `1` has not been used, so we just accept it. However, for the second file, we see that ID `1` is already used, so we have to mangle it to the next available ID (say file one had 5 vehicles, so 1 becomes 6 when reading the second file). Or some kind of mechanism like that to ensure users don't have to manually change IDs. An alternative would be to append a letter `a` for the first file, `b` for the second, etc. So we'd have IDs `1a`, `2a`, ....`na` for the first file, then `1b`, `2b`, ..., `nb` for the second file, etc.

My latest push prepends the file number to the agents' ids. For example, the third vehicle in the second file has vid=23. This ensures uniqueness of ids in the merged scenario that is loaded.

scott-larter commented 3 years ago

This does not seem to work. I tried this command

$ ./GSServer.py -s scenarios/test_scenarios/gs_straight_lateral_demo.osm -s scenarios/test_scenarios/gs_straight_drive -s scenarios/test_scenarios/gs_straight_obstacles.osm -s scenarios/test_scenarios/gs_straight_follow.osm

and I only see vehicle 11 and 12 but they are from the last scenario gs_straight_follow.

This is because you only need one -s flag with the scenarios listed after it. Like this:

./GSServer.py -s scenarios/test_scenarios/gs_straight_lateral_demo.osm scenarios/test_scenarios/gs_straight_drive.osm scenarios/test_scenarios/gs_straight_obstacles.osm scenarios/test_scenarios/gs_straight_follow.osm

However, I just tried the above command and it immediately ends the server because there's a collision.

Also make sure you have .osm for each file. It was missing from gs_straight_drive in your example.

mchlswyr commented 3 years ago

Changes

Testing

mantkiew commented 3 years ago

@mchlswyr Have you also tested with just a single file?

mchlswyr commented 3 years ago

@mchlswyr Have you also tested with just a single file?

Yes, that was the second test.

This should only happen when multiple osm files are loaded.

I think we will have to change this here also. When running a single scenario with vid 1, vid 11 is shown. I think as long as users understand that the file position is prepended, it shouldn't be a problem.

mantkiew commented 3 years ago

@rodrigoqueiroz is that ok for you if the IDs are changed even when a single file is loaded? ID=1 becomes 11.

rodrigoqueiroz commented 3 years ago

@rodrigoqueiroz is that ok for you if the IDs are changed even when a single file is loaded? ID=1 becomes 11.

Changing IDs is not good.

We can assume that combining scenarios is an optional feature and the user is aware of how the merge will change the IDs. However, if the user does not use the merging system, the IDs should be kept as defined.

Suggestion: keep at least the first file (the main one with scenario configuration) with IDs preserved. From the 2nd file onwards, keep the remaining IDs preserved is there is no conflict. Change only when a conflict is detected, and output a Warning message.

We can also have vehicles in the OSM files with ids marked as * (meaning they are not important and can have the ID generated).

scott-larter commented 3 years ago

Suggestion: keep at least the first file (the main one with scenario configuration) with IDs preserved. From the 2nd file onwards, keep the remaining IDs preserved is there is no conflict. Change only when a conflict is detected, and output a Warning message.

We can also have vehicles in the OSM files with ids marked as * (meaning they are not important and can have the ID generated).

What if we have a separate field/attribute for the file number. This way we could leave all the ids the way they are in their individual scenarios and combine the id with the file number whenever we need to create a unique identifier (e.g. in the vehicles dictionary in SimTraffic ).

We might need an extra check in some places to check for the file number, but at least we would only be adding something instead of changing an established and integrated feature like the agent ids.

mchlswyr commented 3 years ago

I reverted the PLOT_VID changes since we aren't prepending the file number anymore.

For testing, I removed the vid tag in scenarios/test_scenarios/gs_straight_pedestrian.osm and in scenarios/test_scenarios/gs_straight_obstacles.osm. Then I ran

python3.8 GSServer.py -s scenarios/test_scenarios/gs_straight_lateral_demo.osm scenarios/test_scenarios/gs_straight_pedestrian.osm scenarios/test_scenarios/gs_straight_obstacles.osm

The vehicles from gs_straight_lateral_demo, gs_straight_pedestrian, and gs_straight_obstacles had ids 1, 2, and 3 respectively.

Next I added the vid tag back to scenarios/test_scenarios/gs_straight_obstacles.osm and ran

python3.8 GSServer.py -s scenarios/test_scenarios/gs_straight_lateral_demo.osm scenarios/test_scenarios/gs_straight_obstacles.osm

The vehicle from gs_straight_obstacles was present, and the vehicle from gs_straight_lateral_demo was not since both scenarios had vehicles with a vid of 1, and the last scenario overwrote the first. Is this the expected behaviour? Or should one of the vehicle ids be incremented?

mantkiew commented 3 years ago

@mchlswyr I think we have to report an ID conflict error and shutdown. These will be rare moving on but we cannot continue with an invalid combination.