Refining task api, etc #63

kywch · 2023-06-03T19:08:54Z

separated Predicate (predicate_api.py) and Task (task_api.py)
- Predicate evaluates progress toward goal and returns float [0,1]
- Task computes rewards for agents and tracks goal completion
refactored task api, allow two methods to provide tasks to the env
- env.reset(new_tasks = a list of task instances)
- env.reset(teams: Dict[int,List[int]], task_spec = a list of tasks to instantiate inside the env)
all the tests are using the new task api
merged the team helpers into lib/team_helper.py
added event number and gold constraints to predicate
fixed IMMORTAL bug (mainly for performance testing)
fixed test_render_save.py, using the new replay helper

daveey · 2023-06-03T20:11:40Z

nmmo/core/env.py

+        map_id: Map index to load. Selects a random map by default
+        seed: random seed to use
+        new_tasks: A list of instantiated tasks
+        task_spec: A list of task spec to instantiate inside reset()


how about just make_new_tasks_fn

that way env doesn't need to know anything about making tasks, or teams

Can you please tell me more about it? Let me think about it again after sleep.

However, I also realized that the current make_team_tasks(teams, task_spec) doesn't need any info about the env. If one can pass in teams and task_spec, one can also use make_team_tasks to make task instances and pass them in. ... Then yes, the env doesn't need to know anything about making tasks or teams.

Oh, I got it, and implemented as follows. But after writing it, I just thought... if one knows it all, why bother passing in these and not the instantiated tasks? Please let me know what you think.

if new_tasks is not None: # providing an empty new_tasks [] is also possible self.tasks = new_tasks elif make_task_fn is not None: self.tasks = make_task_fn(**make_task_fn_kwargs) else: for task in self.tasks: task.reset()

daveey · 2023-06-03T20:14:29Z

nmmo/core/env.py

@@ -88,12 +78,6 @@ def box(rows, cols):
    if self.config.PROVIDE_ACTION_TARGETS:
      obs_space['ActionTargets'] = self.action_space(None)

-    if self._task_encoding:


don't we need this somewhere?

We need it, but not necessary in this format. Let me check back after seeing the syllabus integration.

daveey · 2023-06-03T20:17:39Z

nmmo/core/env.py

+    elif task_spec is not None and teams is not None:
+      self.tasks = make_team_tasks(teams, task_spec)
+    else:
+      self.tasks = nmmo_default_task(self.possible_agents)


this is going to replace existing tasks with the default_task if we don't pass in new_tasks, which is not expected. how about we set self.tasks to default_task in the constructor, and then don't do that here

nmmo_default_task has moved to the init, and this line was replaced with

for task in self.tasks: task.reset()

not changing the existing tasks.

daveey · 2023-06-03T20:18:10Z

nmmo/core/env.py

@@ -308,11 +283,7 @@ def step(self, actions: Dict[int, Dict[str, Dict[str, Any]]]):

    # Store the observations, since actions reference them
    self.obs = self._compute_observations()
-    gym_obs = {}
-    for a, o in self.obs.items():


don't we still need this?

Previously, this was gym_obs = {a: o.to_gym() for a,o in self.obs.items()}, but it was changed to that to add in

if self._task_encoding: gym_obs[a]['Task'] = self._encode_goal().get(a,np.zeros(self._task_embedding_size))

Currently, I'm not sure about the exact form of the task encoding. Hoping to get some input/specs as we start task conditioned learning very soon

daveey · 2023-06-03T20:19:05Z

nmmo/systems/skill.py

    depletion = config.RESOURCE_DEPLETION_RATE
    water = self.entity.resources.water
    water.decrement(depletion)

-    if self.config.IMMORTAL:


i don't love IMMORTAL, maybe we can just get rid of it?

IMMORTAL is pretty much for performance testing. Perhaps change to PERFORMANCE_TEST?

daveey · 2023-06-03T20:19:58Z

nmmo/task/base_predicates.py

@@ -43,6 +43,11 @@ def StayAlive(gs: GameState,
  """True if all subjects are alive.
  """
  return count(subject.health > 0) == len(subject)
+  # The below is for speed testing (bypass GroupView)


let's not include it in this PR, perf testing can go in a different PR if we want to commit it at all

daveey · 2023-06-03T20:20:31Z

nmmo/task/base_predicates.py

@@ -87,7 +92,7 @@ def CanSeeGroup(gs: GameState,
                target: Group               = constraint.TEAM_GROUPS):
  """ Returns True if subject can see any of target
  """
-  return OR(*(CanSeeAgent(subject, agent) for agent in target.agents))
+  return POR(*(CanSeeAgent(subject, agent) for agent in target.agents))


why POR not OR?

OR for Predicate. But I hear you, and am removing all P prefix.

daveey · 2023-06-03T20:27:31Z

nmmo/task/predicate_api.py

+
+    return task_cls(eval_fn=self, assignee=assignee, reward_multiplier=reward_multiplier)
+
+  def __and__(self, other):


not a fan of the P prefix

daveey · 2023-06-03T20:28:12Z

nmmo/task/predicate_api.py

+    return POR(self, other)
+  def __invert__(self):
+    return PNOT(self)
+  def __rshift__(self, other):


IMPLY seems really weird for task predicates, do we really need it?

Simple Optimizations for Speed V2

minor changes to predicate api and caching

daveey · 2023-06-05T20:03:53Z

nmmo/core/env.py

-                     embedding_size=self._task_embedding_size,
-                     task_encoding=self._task_encoding,
-                     reset=False)
+    self.tasks = nmmo_default_task(self.possible_agents)


could you do task_api.make_nmmo_default_tasks()

explicitly stated where the default task comes from.

daveey · 2023-06-05T20:05:22Z

nmmo/core/env.py

-  def reset(self, map_id=None, seed=None, options=None):
+  def reset(self, map_id=None, seed=None, options=None,
+            new_tasks: List[Task]=None,
+            make_task_fn: Callable=None,


we only need make_task_fn, the caller can define a lambda without having to pass kwargs. and they can always return [] if the want to clear the tasks. there's no need to accept new_tasks

David, you suggest that we get rid of new_tasks, a list of task instances, and go only with make_task_fn, which can be

make_task_fn = lambda: return make_team_tasks(teams, task_spec) in case of the manual curriculum example

or make_task_fn = 'lambda: return []'

and inside reset(), self.tasks = make_task_fn() is called. Am I right?

@jsuarez5341 how would the puffer like to handle this?

I got the part that we don't need to pass in args for make_task_fn, so removed it.

And, I agree that new_tasks and make_task_fn are redundant. Well, ok ... it's pretty easy to put new_tasks in, so I'll go ahead only with make_task_fn.

daveey · 2023-06-05T20:07:38Z

nmmo/core/env.py

+
+    # Remove rewards for dead agents
+    for agent_id in dones:
+      rewards[agent_id] = -1


this seems wrong? dones only contains agents who died this turn. seems fine for them to get a reward. but agents that have already been dead should not be rewarded after death, right?

or... maybe they should be, if their task is accomplished after they died

I see you are tinkering with this part too. I don't have any strong idea, so I'll just take it out.

daveey · 2023-06-05T20:08:00Z

nmmo/core/realm.py

@@ -104,6 +101,10 @@ def reset(self, map_id: int = None):
    Item.INSTANCE_ID = 0
    self.items = {}

+    if self._replay_helper is not None:
+      self._replay_helper.reset()
+      self._replay_helper.update() # capture the initial packet


should we just call update() in reset?

kywch added 8 commits June 2, 2023 04:49

refining task api

1a578a6

added create_task() to predicate

e16995a

added make_team_tasks(), init tasks in reset, etc

977f6b0

can pass function into reset to create task

a7975e5

fixed typo

62f3e0b

corrected team spawn pos, left/right team

ad1bc2b

refactored nmmo_default_task()

58dd1d7

refactored team_helper, checked constraints

153a1ed

daveey requested changes Jun 3, 2023

View reviewed changes

nikhilpinnaparaju and others added 9 commits June 3, 2023 21:43

Simple Optimizations for Speed V2

fc3ae27

fixing import order

b40bf91

Merge pull request #65 from CarperAI/optim

85b699f

Simple Optimizations for Speed V2

tweaking how to pass tasks to env, removed P prefix

1d2d46e

minor changes to predicate api and caching

25bab85

Merge pull request #66 from CarperAI/optim

20db562

minor changes to predicate api and caching

created manual curriculum, tweaked task api

18b04de

curriculum bug fix, add reset packet to replay

4f0c94c

merge branch '2.0' into task-rev

44b47c1

daveey requested changes Jun 5, 2023

View reviewed changes

kywch added 5 commits June 5, 2023 22:15

env.reset() only takes make_task_fn

d9aa89b

Merge branch '2.0' into task-rev

decb0a7

updated nmmo_default_task(), profiled task system

332f843

clean up make-task helpers

44eab79

renamed the make task from task spec example

79a66bd

jsuarez5341 merged commit 7386079 into 2.0 Jun 7, 2023
6 checks passed

kywch deleted the task-rev branch June 9, 2023 06:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refining task api, etc #63

Refining task api, etc #63

kywch commented Jun 3, 2023

daveey Jun 3, 2023

kywch Jun 4, 2023

kywch Jun 4, 2023

daveey Jun 3, 2023

kywch Jun 4, 2023

daveey Jun 3, 2023

kywch Jun 4, 2023

daveey Jun 3, 2023

kywch Jun 4, 2023

daveey Jun 3, 2023

kywch Jun 4, 2023

daveey Jun 3, 2023

kywch Jun 4, 2023

daveey Jun 3, 2023

kywch Jun 4, 2023

daveey Jun 3, 2023

kywch Jun 4, 2023

daveey Jun 3, 2023

kywch Jun 4, 2023

daveey Jun 5, 2023

kywch Jun 5, 2023

daveey Jun 5, 2023

kywch Jun 5, 2023

kywch Jun 5, 2023

daveey Jun 5, 2023

kywch Jun 5, 2023

daveey Jun 5, 2023

kywch Jun 5, 2023


		return task_cls(eval_fn=self, assignee=assignee, reward_multiplier=reward_multiplier)

		def __and__(self, other):

Refining task api, etc #63

Refining task api, etc #63

Conversation

kywch commented Jun 3, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment