Ran into a problem using priorities on process actions. Here's the scenario.
Empty plane lands at empty airport and refuels with low priority (just because there is nothing better to do)
During refuel a new cargo is available at the airport
The next process action requests to load the cargo at a high priority
However, there is an existing action in the queue with same agent but different priority. This generates a "The agent is not next in queue!" warning. Unfortunately, the agent is never processed the cargo never pickup up.
I can write extra code to prevent priority changes in generated actions, but "priority change bricks the agent" is annoying. So any help on your end would be appreciated.
Posted by: jkolen @ Nov. 30, 2023, 2:16 p.m.hi jkolen,
Just to clarify, do you want to update the agent's priority while its already in the queue? (Since a new cargo appeared, but are unable to do so because the agent was already added in a much lower priority)
Thank you
Posted by: adelanovic @ Nov. 30, 2023, 4 p.m.It would be nice to change the priority for pending actions. Keeping with the initial priority would be ok, as well, which I think is the intent described in the documentation. This decision should be on your team as it's a world modelling issue.
What needs to be fixed is the sending a different priority once the agent is in the queue prevents the agent from being serviced, ever.
After more digging on my end, I found that refuelings were assigned priorities based on state. If the state changed, like when a new cargo was locally available, priority changed. I changed refuel actions to always have priority None (the noop action), and the problem went away. I would like to specify refuel priority and not brick the aircraft because I changed the priority.
As far as I can tell, this is rooted in agents.py where the next in queue is returned as a priority/EnvAgent pair and is compared with the current agent as a priority/EnvAgent. With different priorities, processing the agent is skipped because it's not next available.
Posted by: jkolen @ Nov. 30, 2023, 5:42 p.m.Hello jkolen,
Thank you for raising this issue and the detailed explanation. I'm sorry for the troubles.
We'll plan on having a fix out tomorrow (Friday)...
Thank you
Posted by: abeckus @ Nov. 30, 2023, 8:58 p.m.Hi jkolen,
We made a choice to allow the updating of priorities and in turn this should also make sure that agents are not lost to the void. The update is on our github page, let me know if you still experience this issue.
Thanks for the feedback as always
Posted by: adelanovic @ Dec. 1, 2023, 6:23 p.m.Just to add to this jkolen, I noticed your submission was timed out after 2hours. This was increased back to 3hrs allowed execution time.
Posted by: adelanovic @ Dec. 1, 2023, 6:56 p.m.Thanks for increasing the time and the bug fixes. I'm still seeing odd behavior on the queues. I had a priority 1 agent passed up for processing by priority 3. I assumed that low number processed first. If that's not the case, please let me know.
Also, there are still some issues with next_in_queue in the try_to_process method. Only the first comparison was fixed to next_in_queue[2]. There are two more comparisons involving next_in_queue that always fail because a tuple is not an instance of EnvAgent.
Even with those fixes locally, it's still is not behaving as expected.
After looking at the airplane queue code. It makes sense what I was seeing.
Posted by: jkolen @ Dec. 4, 2023, 1:31 a.m.hi jkolen, is the problem with the queue order all set? Or do we need to look into it further? As always, thank you for the feedback. I'll checkout the self/next_in_queue checks.
Posted by: adelanovic @ Dec. 4, 2023, 1:31 p.m.There is still a problem with changed priorities.
In try_to_process():
check_agent_in_queue, queue_item = self.current_airport.agents_waiting.is_agent_in_queue(self)
# If the agent is in the queue and the priority was updated.
if check_agent_in_queue and action['priority'] != self.priority:
# Update the queue
airport_counter = self.current_airport.counter
self.current_airport.agents_waiting.update_priority(self.priority, action['priority'], queue_item[1], airport_counter, self)
This looks good. However, in step():
if action['priority'] is not None:
self.priority = action['priority']
and then it calls the handlers. So by the time it reaches try_to_process, the condition will never fire.
This seems to work:
def step(...)
...
if action['priority'] is not None:
self.priority_changed = (self.priority == action['priority'])
self.priority = action['priority']
else:
self.priority_changed = False
def try_to_process(...)
...
check_agent_in_queue, queue_item = self.current_airport.agents_waiting.is_agent_in_queue(self)
# If the agent is in the queue and the priority was updated.
if check_agent_in_queue and self.priority_changed:
# Update the queue
airport_counter = self.current_airport.counter
self.current_airport.agents_waiting.update_priority(self.priority, action['priority'], queue_item[1], airport_counter, self)
typo: self.priority != action['priority']
Posted by: jkolen @ Dec. 4, 2023, 11 p.m.You will also need to keep the old priority around as well.
Posted by: jkolen @ Dec. 5, 2023, 1:31 a.m.jkolen,
Just to give you an update:
Thank you for posting this fix, that makes sense.
We are still looking at the code - we want to do some additional investigation/testing to make sure the priority queue is working before releasing the fix....
Thank you
Posted by: abeckus @ Dec. 5, 2023, 7:51 p.m.Also noticed that the return false condition is indented incorrectly in is_agent_in_queue
Posted by: jkolen @ Dec. 6, 2023, 3:18 a.m.Hello jkolen,
We pushed a fix for the priority queue this afternoon and just finished testing - the simulator code has been pushed to Github and the CodaLab evaluator updated (your latest submission ran with the new simulator code).
This addresses your comment on December 4th regarding the priority update never firing (we ended up storing the previous priority in the agent object in order to check for a change), as well as the most recent comment regarding the is_agent_in_queue method (thank you for catching that - we also found it during testing).
We also added a new unit test for more through testing of the agent's priority logic and made a few other tweaks.
Thank you again and sorry for the difficulties
Posted by: abeckus @ Dec. 7, 2023, 12:59 a.m.