Wrong future stop tick predictions 馃悰 #384
Comments
|
I think that the behavior is still full of bugs. A first fix is given by the following: However, error still occurs, Returns: As you can see, at some point, the chain is broken and wrong predictions are returned. |
|
Update: I figured out where the problem is. The problem arises when the same port appears multiple times in the same route. retrieves always the first time that port appears. |
|
Note that this problem seems not to be fixable with the current signature of the method: from vessel_idx, last_port_idx, last_port_arrival_tick and stop_number it is impossibile to retrieve correctly the next position in the route. An option would be the one of inserting a new attribute in the vessels that stores the correct position within the route, so that it is would be very easy to understand its position. To summarize, the problems are:
|
riccardopoiani
changed the title
|
Here is the fix I'm applying. Of course you need to modify |
|
Hi @riccardopoiani , thank you for your report & nice bug fix! The first thing that I would like to talk about is the expectation of the "vessels' future tick matrix". You have mentioned that all values in this matrix should be greater than the corresponding "current" tick. However, under current logic, this is not the fact. The "future tick" may be smaller than the "current tick" due to the inaccuracy of the prediction. For example, suppose we make a prediction at The bug you found does exist, but I would say it is another unrelated matter. The "time travel" will still exist even if replacing the buggy function with the new one you proposed (you could have a try and see if you can get this result). In my personal understanding, under current logic, a vessel's future tick vector is meaningful only at the ticks that have corresponding events (e.g., arrival, departure). The information may be incorrect between two adjacent events, and it is fine. There are also other possible ways to improve this issue. For example, once we notice that the "future tick" is behind the "current tick", we could know that the "future tick" is wrong and outdated, and we can make a new prediction accordingly. We will discuss this issue later. |
|
@riccardopoiani The second thing I would like to talk about is your bug fix. In your bug fix, you compare the following route with the following vessel plan to check if the chosen port is the right one. The length being compared is equal to One corner case I could think of is the repeated pattern in the route. For example, if a route is In my opinion, we could use another simple way to solve this issue. We know the number of ports that the vessel has already travelled ( The following is my modification. See if it is better, thanks! def _get_vessel_start_port_offset(self, vessel_idx: int) -> int:
vessel = self._vessels[vessel_idx]
route_points = self._routes[self._route_mapping[vessel.route_name]]
route_point_names = [rp.port_name for rp in route_points]
vessel_start_port_name = self._vessels[vessel_idx].start_port_name
vessel_start_port_offset = route_point_names.index(vessel_start_port_name)
return vessel_start_port_offset
def _predict_future_stops(self, vessel_idx: int, last_stop_idx: int, stop_number: int):
"""Do predict future stops.
"""
vessel = self._vessels[vessel_idx]
speed, duration = vessel.sailing_speed, vessel.parking_duration
route_points = self._routes[self._route_mapping[vessel.route_name]]
route_length = len(route_points)
last_stop = self._stops[vessel_idx][last_stop_idx]
last_port_arrival_tick = last_stop.arrival_tick
vessel_start_port_offset = self._get_vessel_start_port_offset(vessel_idx)
last_loc_idx = (vessel_start_port_offset + last_stop_idx) % len(route_points)
predicted_future_stops = []
arrival_tick = last_port_arrival_tick
# predict from configured sailing plan, not from stops
for loc_idx in range(last_loc_idx + 1, last_loc_idx + stop_number + 1):
next_route_info = route_points[loc_idx % route_length]
last_route_info = route_points[(loc_idx - 1) % route_length]
port_idx = self._port_mapping[next_route_info.port_name]
distance_to_next_port = last_route_info.distance_to_next_port
# NO noise for speed
arrival_tick += duration + ceil(distance_to_next_port / speed)
predicted_future_stops.append(
Stop(
-1, # predict stop do not have valid index
arrival_tick,
arrival_tick + duration,
port_idx,
vessel_idx
)
)
return predicted_future_stops |
Hi @lihuoran , you are completely right about this point. I agree that I consider acceptable for the moment to have some wrong and outdated estimates; minor fixes such as the one that you are suggesting at the end are not that crucial to me and I think it is ok to have "wrong" estimates. |
You are right and this solutions seems to be definitely better. I did some rapid tests by hand and it seems to be working. |
|
@riccardopoiani The update mentioned above has already been merged into master (with slight modification). Thanks for your efforts! |
Description
There is a strange behavior in
future_stop_tick_listattributes.As you can see, the current tick is
100, and yet there are future vessel tick predictions that are way behind the current tick, for instance[ 75., 88., 96.].I would expect the snapshot to retrieve ticks that are always ahead of the current one.
To Reproduce
Here you can find that output that I had:
Expected Behavior
As you can see, the current tick is 100, but some vessels future ticks are completely in the past.
I would expect this matrix to contain all information that are > 100.
Environment
CIM,Citi Bike): CIMSimulation,RL,Distributed Training): SimulationGraSS on Azure,AKS on Azure):pip,source): sourceLinux,Windows,macOS):3.6,3.7): 3.7Additional Context
EDIT: The problem seems to affect also global configurations without noise in the vessels speed.
E.g. try this config:
The text was updated successfully, but these errors were encountered: