Bug: NPC agents seen as user agents (isNPC=False) in multiple scenrio

[MoAljam]

example: In Space Sick Normal and Challenge seed 0-5 all NPC agents are handled as user agents, with dedicated UIs.
this prevents relying on the API to differentiate and control user-agents for subsequent tasks.

The issue seems to be due to faulty initialization in the NPC-agent classes that are sub-classed from NPC(Agent). Theses classes call Agent.init(...), skipping NPC(Agent) which sets the attribute of isNPC to true.

[PeterAJansen]

Hi @MoAljam ,

Thanks for this issue report, and we'll investigate.

It sounds like this would only be potentially blocking in cases where the user had instantiated multiple user agents. Is this a scenario that you're in?

best wishes,
Peter

[MoAljam]

Hi @PeterAJansen

yes exactly. I am loading the scenario with multiple user agents (n<5) through 'DiscoveryWorldAPI.loadScenario'.

thanks for the quick response.
best,
Mohamad

[PeterAJansen]

Thanks -- I think I understand the problem better -- the user-facing API enumerates the user agents with world.getUserAgents(), and since that checks agent.attributes["isNPC"], and the NPCs are currently inheriting from Agent() rather than NPC(), then it returns all the agents -- so you're not sure which ones you can control for the one scenario that does have NPCs (Space Sick), and also potentially for the Soil Controller and Crystal Reactors, which might enumerate as NPCs since they're dialogable. Sorry about that!

Solution 1 (Quick):

The easiest (quick) fix would be to clone a local copy of the DiscoveryWorld repo, and add this single line:

self.attributes["isNPC"] = True

To each of the NPC constructors:

NPCChef:

discoveryworld/discoveryworld/Agent.py

Line 1683 in fd59132

class NPCChef(NPC):

NPCColonist:

discoveryworld/discoveryworld/Agent.py

Line 1970 in fd59132

class NPCColonist(NPC):

NPCColonist1:

discoveryworld/discoveryworld/Agent.py

Line 2130 in fd59132

class NPCColonist1(NPC):

NPCChef1:

discoveryworld/discoveryworld/Agent.py

Line 2235 in fd59132

class NPCChef1(NPC):

NPCColonistAuto2:

discoveryworld/discoveryworld/Agent.py

Line 2429 in fd59132

class NPCColonistAuto2(NPC):

NPCFarmer1:

discoveryworld/discoveryworld/Agent.py

Line 2609 in fd59132

class NPCFarmer1(NPC):

SoilController:

discoveryworld/discoveryworld/Agent.py

Line 2809 in fd59132

class SoilController(NPCDevice):

CrystalReactor:

discoveryworld/discoveryworld/Agent.py

Line 2933 in fd59132

class CrystalReactor(NPCDevice):

This should fix the enumeration for world.getUserAgents(), and get you running quickly.

Solution 2 (Longer-term):

Longer-term, it would make sense to have the NPCs call the proper constructor (NPC() instead of Agent()). But the NPCs were quite difficult to debug, and I'm slightly hesitant to recommend this switch since it might introduce regressions that are hard to find, and the current NPCs are well-debugged now with the current constructor (except for this bug).

How does that sound?

[MoAljam]

Sounds good, thanks for the solution suggestions.
I agree on the quick fix, it would be the simplest and most consistent. however I would like to avoid changes on the package directly to keep the project more portable.
for now, I'm using a simple wrapper to infer the isNPC attribute (isinstance(some_agent, NPC) ). It seems to resolve the issues consistently, both for the NPC-subclassed agents and the dialogable ( Soil Controller and Crystal Reactors ).

[PeterAJansen]

Thanks -- we'll try to incorporate this (or a similar) change in the next release, so that hopefully portability/etc won't be an issue for you.

It's very exciting that you're working on a multi-agent implementation for DiscoveryWorld. We added this capability into the environment at the design stage in the hopes that folks would use it for shared memory agents/other interesting implementations, but I haven't seen any implementations yet. Please send a note when you have finished to let us know about your work and the findings!