restructure brain interface

[bjuergens]

the current ctrnn class is a convoluted mess. Mainly because I implemented stuff quick&dirty without regard for code quality and maintainability.

In this issue I want to collect and discuss ideas for improving this. I also want to collect concrete problems with this code (i.e. violations to specific software design principles, e.g. SOLID). And I want to collect possible features that could be added later, and would be affected by the refactoring

[bjuergens]

I think the main issue with the class ContinuousTimeRNN is a violation of Single-responsibility principle. The class handles both the concrete instance of a brain (which is used by the episode runner) and the general model of a brain (initialized by the Experiment and by worker threads). All @classmethod are used for the latter function. Most remaining functions are used for the former. And because these methods are used by outside class, they must also be present in the public interface of IBrain.

The class is also open for side-effects, because the class variables could change during runtime, which would break future brains. Also there is no guarantee that the class state is persistent between workers, if it should ever change.

Also the class state needs to be initialized on worker threads. for Dask this happens here. For other frameworks this happens either by fork-magic (e.g. SCOOP) or not at all (e.g. MP).

The Brain class is known to the workers-handlers and to the experiment, but these two classes never initialize a concrete object from it, and only set up the class state. The Brain class known to the episode runner, which ignores most parts of the class's public interface and only uses methods on the specific object. The fact that the experiment class has access to the interface of the entire class, but doesn't use/need it, is an example of bad code complexity.

---

The methods which are needed by the episode runner are:

• __init__()
• step()
• _learned_sparse() (private, only used in a specific configuration which may even be removed, I think)

The methods which are used to set up the class state are:

• get_class_state
• set_class_state
• get_individual_size
• set_masks_globally
• _generate_mask

I propose to split this class in two. The concrete brain itself (which is initialized and used by the episode runner). And a template/factory-class (which is currently implemented as the class state).

For this I would simply move all methods related to the current setting up of the class state to the new template-class. Here these information will not be stored in the class state, but instead a template-object will be created. The template will have a method which takes a genome and returns a specific brain. This template object will then once be passed to the episode-runners one each worker. The episode runner will then use this template to create the brain from the genome. The template object will be read-only (, i.e. frozen, slots and immutable), to avoid side effects later.

With this the date flow would be like this: First experiment.py creates a template-object. The template object is passed to the MPHandler. The MPHandler creates a eprunner-object for each worker, and to the eprunner a template-object is attached (along with a gym-object btw). When the eprunner's evaluate-method it called with a genome, it sends the genome to its template-object to create a specific instance of a brain from it.

---

@pdeubel was hältst du von der Idee?

Das sollte eigentlich alles lösen, was wir gestern besprochen haben, oder? (sogar die sache wo der eprunner das brain zwischen den läufen speichert und nur die gewichte ändert)

Ist es verständlich, was ich hier ausdrücken will? Wäre es sinnvoll, wenn ich das irgendwie mit diagrammen visualisieren würde? (Wenn Julian mit sein BA anfängt, wird er das hier verstehen müssen, weil er ja selbst auch neue brains implementieren muss)

[bjuergens]

@DanielZim meinst du es wäre sinnvoll, wenn Julian hierzu auch seine meinung teilen würde?

[DanielZim]

Ja das können wir gerne am Montag zusammen mit Julian mal besprechen

[pdeubel]

@pdeubel was hältst du von der Idee?

Finde das Aufteilen der Klasse gut, war mir ja auch direkt in den Sinn gekommen. Allerdings wäre es dann so dass jeder Worker ein eigenes EpisodeRunner Objekt hat? Das würde ja bei dem multiprocessing.Pool nicht gehen (man hat keinen Zugriff auf die einzelnen Worker, hier war die Lösung eventuell multiprocessing.Process zu sub-classen aber das finde ich nicht so schön).

Dann noch eine Frage zum Verständnis: Angenommen man würde was am Template des Brains ändern, wie wird das dem Template-Objekt in jedem Worker mitgeteilt? Das ist fest für die Lifetime des Workers, oder?

Kann man das Template-Objekt nicht irgendwie mit der Template-Klasse als Singleton realisieren? Wenn man das macht bevor das parallel processing gestartet wird haben alle Worker Zugriff darauf.

[bjuergens]

Angenommen man würde was am Template des Brains ändern, wie wird das dem Template-Objekt in jedem Worker mitgeteilt?

Man muss sich dann einen Mechanismus überlegen, um das neue Object and die Worker zu verteilen. Was mMn einfacher ist, wenn das Template in einem expliziten Object liegt, anstatt (so wie im Moment) an den globalen state der Klasse gebunden ist.

Man könnte das auch so machen, dass das Template-Object gar nicht an die Worker verteil wird, und statt dessen nur die fertigen brains direkt an die worker verteilt wird. Also der optimizer macht dann quasi nicht map(evaluate, genomes), sondern map(evaluate, [template.make(g) for g in genomes])

uh, und das muss eigentlich auch gar nicht der optimizer machen, sondern das könnte dann der MPHandler machen. Und dann kann man an der stelle eine unterschiedliche behandlung für die Dask und MP machen, um jeweils die vorteile von beiden voll auszunutzen, ohne dass der rest des codes komplexer wird.

Also der MPHändler wurde dann in seiner batch_evaluate Methode die brains initialisieren bevor sie zum worker geschickt werden, während Dask nur die genome an die worker schickt. Das würde dann etwa so aussehen:

im MPHandler:

def _doWork(brain, seed):
    get_worker().eprunner.evaluate(brain,seed)

def batch_evaluate(self, genomes, seeds)
    brains = [self.template.make(g) for g in genomes]
    return self.map(self, _doWork, brains, seeds)

im DaskHandler:

def _doWork(genome, seed):
    brain = get_worker().template.make(genome)
    get_worker().eprunner.evaluate(brain,seed)

def batch_evaluate(self, genomes, seeds)
    return cls._client.gather(cls._client.map(self, _doWork, genomes, seeds))

[pdeubel]

Ok, ja das gefällt mir, so ähnlich hatte ich mir das auch am Freitag gedacht :+1:

Für das get_worker().eprunner im MPHandler überleg ich mir was wie man das geschickt machen könnte

[pdeubel]

multiprocessing.Process zu sub-classen damit man z.B. ein Attribut eprunner halten kann geht nicht. Wenn multiprocessing.Process einmal ausgeführt wurde dann wars das, das heißt für mehrmaliges Ausführen muss man einen neuen multiprocessing.Process erstellen. Wäre blöd dann wird jedes mal der EpisodeRunner neu erstellt und damit auch die Environment.

Eine andere Möglichkeit wäre den bestehenden multiprocessing.Pool zu nutzen und im MPHandler.py zusätzlich ein dict mit den Prozess IDs als Key und jeweils einem EpisodeRunner Objekt als Value zu haben. Das heißt jeder Worker Prozess des Pools könnte sich dann über seine ID immer seinen EpisodeRunner holen.

Das ist aber recht umständlich:

1. Bin ich mir nicht sicher ob die Worker Prozesse tatsächlich über die gesamte Laufzeit des Trainings gleich bleiben, oder ob der Pool irgendwann mal aufräumt und neue Prozesse startet. Dann würde sich die PID ändern -> schlecht. Ich hab zu der exakten Lifetime bisher nicht wirklich was gefunden.
2. Der Pool hat keine Funktion um die PIDs auszulesen, man muss also sowas wie [pool.apply_async(os.getpid()) for _ in range(number_of_workers)] aufrufen nachdem der Pool erstellt wurde, damit der MPHandler die PIDs kennt. Da könnte es sein dass manche Prozesse doppelt aufgerufen werden, wenn z.B. der erste aufgerufene Prozess schon fertig ist, wäre er wieder bereit im Pool und könnte dann nochmal seine PID eintragen. Hier gibt es vielleicht eine bessere Möglichkeit.

Also bisher eher kompliziert, würde ich ehrlich gesagt nicht so implementieren. Vielleicht fällt mir noch was besseres ein.

[bjuergens]

würde das nicht für MP funktionieren? Der MP-Worker würde niemals das Template benutzen müssen und das Problem hat sich erübrigt.

• 

Man beachte: Die signatur von "batch_evaluate" ist in beiden Fällen gleich. Die Signature von "_doWork" ist jedoch unterschiedlich.

[pdeubel]

get_worker().eprunner ist das Problem -> Es gibt keine get_worker() Funktion vom multiprocessing.Pool direkt und wenn man sie selbst schreibt muss man das mit den PIDs machen. Und wenn man die get_worker Funktion hätte, kann man den Workern kein eprunner Objekt geben

[bjuergens]

verstehe

wäre es so schlimm, wenn bei MP einfach jedesmal eine frische env angelegt werden würde?

Ich kann mir vorstellen, dass es bei einigen envs keinen unterschied macht (z.B. bei atari envs ist das reseeden der env genauso teuer wie das neuanlegen iirc). Und wenn es doch einen signifikanten unterschied macht, dann kann man für das jeweilige experiment einfach einen anderes Multiprocessing framework benutzen.

andererseits: solange die zusätzliche komplexität den MPHandler nicht verlässt, halte ich das auch für ok. Die Frage, die ich hier sehe ist nur, ob der geschwindigkeitsvorteil hoch genug ist um den implementierungsaufwand zu rechtfertigen.

[bjuergens]

relevanter bit an information bezüglich des template-singletons

Note: Singleton classes are not really used as often in Python as in other languages. The effect of a singleton is usually better implemented as a global variable in a module.

https://realpython.com/primer-on-python-decorators/#creating-singletons

[Dranero]

Ich noch nicht ganz durch Dask durch, aber könnte man nicht das Enviroment beim Worker Setup erstellen? https://docs.dask.org/en/latest/setup/custom-startup.html Dann muss man dass nur noch vor jedem Durchgang resetten (Ich glaube das ist gerade EpisodeRunner Funktionalität)

Dann würden Worker mit Netz- und Experiment-Config initialisiert werden und das Aufgabenpaket beinhaltet nur noch die Individuals für die Brains.

[pdeubel]

@Dranero Dadurch dass wir neben Dask noch multiprocessing und evtl. ray als Frameworks für das parallel processing nutzen wäre die Lösung denke ich zu spezifisch für Dask

[pdeubel]

@bjuergens Ich würde es so ähnlich implementieren wie du es in dem Kommentar beschrieben hast:

1. Die Brain Klassen aufteilen in BrainTemplate und Brain Klassen wobei die Templates die Masks etc. erstellen und mit einer Funktion create_brain() ein Brain erstellen können welches der EpisodeRunner dann nutzt. Die Templates sind read-only wie du beschrieben hast.
2. Der EpisodeRunner bekommt in der eval_fitness() Methode einen zusätzlichen erforderlichen Parameter template der das Template enthält. Mit dem kann dann das Brain erstellt werden.
3. Das Template wird der train() Methode des Optimizer übergeben sodass in algorithms.py in evaluate_candidates() das Template als neuer Parameter übergeben wird und dann in der toolbox.map() dem EpisodeRunner übergeben werden kann.

Zu 2.: Ich finde es besser wenn der EpisodeRunner und nicht der ProcessingHandler die Brains über das Template erstellt. Es wäre denke ich etwas versteckt das Brain im ProcessingHandler zu erzeugen der eigentlich nur die Berechnung delegieren sollte.

Der Ansatz würde denke ich auch offenlassen das Template nach einer Generation z.B. mal zu ändern. Dazu könnte man zB einen neuen Optimizer schreiben der dann in seiner train() Methode bzw. in der korrespondierenden Methode in algorithms.py das Template Objekt durch ein neues ersetzt.

So würde das denke ich mit allen Parallel processing frameworks funktionieren.

Wie findest du das?

[bjuergens]

anm. (hatte ich gerade ins slack geschrieben, ist hier aber auch relevant)

für netze, die keinen brainstate brauchen würde man eine dummy/default klasse benutzen, die nicht viel komplexität erzeugt. Für bisherige Klassen, die den brainstate brauchen, wird der code durch eine Factory deutlich verständlicher. Und als Bonus haben wir die möglichkeit coolere Factory-Dinge zu tun. z.B. kann die CTRNN-Factory Klasse ein Basis-CTRNN erzeugen, und immer wenn ein neues CTRNN generiert werden muss, dann wird das Basis-CTRNN kopiert, und dann werden einfach nur die Werte des Genomes hineingegossen. Das dürfte dann die Zeit zum initialisieren der CTRNNs deutlich reduzieren.