Skip to content

Conversation

vincentpierre
Copy link
Contributor

@vincentpierre vincentpierre commented Sep 11, 2018

Feature : Remove Graph Scopes

Abstract

As part of our work to make inference within the Unity Engine easier to use, we propose to generate one graph per brain when training with multi-brain rather than a single graph with multiple graphs scopes.

Background

A key project we want to integrate in v0.6 is around inference within Unity. As such, there are several pieces that are required in order to make inference in Unity more intuitive. The first step is to ensure the graphs we generate through our training process are always formatted the same way. This is currently not the case when a model is trained through multi-brain since the graph generated contains multiple inference sub-graphs with different graph scopes.

Goals

  • Make each model a single inference graph (only one action node per .bytes file)
  • Generate only one bytes file per brain when training with multi-brain
  • No disruption of the current workflow

Non-Goals

  • Create new trainers / policies

Plan

The graph generated with multi-brain goes under : models/<run-id>/<env-name>_<run-id>.bytes with brain-name for graph scope
We propose to genetate multiple graphs without graph scopes under models/<run-id>/<brain-name>.bytes.
The policy will be responsible for serializing, saving and loading the models and will be called from the trainer_controller through trainer base class method calls.

Acceptance criteria

Single model per brain when training with multi-brain
Design aligns with long term plan

Milestones

  • Implement a POC for saving sub-graphs into different
  • Review functionality
  • Review Design (@awjuliani)
  • Fix the pytest
  • Write documentation PR here
  • Remove Graph Scope property from Internal Brain PR here

@vincentpierre vincentpierre changed the base branch from develop to master September 12, 2018 17:51
@vincentpierre vincentpierre changed the base branch from master to develop September 12, 2018 17:52
Copy link
Contributor

@awjuliani awjuliani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good. Just remove dead code and comments.

@vincentpierre vincentpierre merged commit 0855eec into develop Sep 17, 2018
@vincentpierre vincentpierre deleted the develop-remove-graph-scope branch September 17, 2018 15:43
@@ -1,9 +1,12 @@
import logging
import numpy as np
import tensorflow as tf
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vincentpierre - I thought we were aiming to remove TF dependency from the Policy class?

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants