neuroevolution-ai / NeuroEvolution-CTRNN_new

MIT License
3 stars 3 forks source link

Procgen2 #46

Closed bjuergens closed 3 years ago

bjuergens commented 3 years ago

WIP

bjuergens commented 3 years ago

@pdeubel ich habe 2 fragen an dich.

Frage 1: Ich will die Möglichkeit haben zu sehen, was genau der agent sieht. In dem Konkreten Fall will ich sehen wie klein ich die auflösung machen kann, sodass da noch alle wichtigen optionen drin sind. Im Commit https://github.com/neuroevolution-ai/NeuroEvolution-CTRNN_new/pull/46/commits/44fa27e78c7569104af94da7f5ce950b00ab9390 habe ich das erst beste implementiert, was mir dazu eingefallen ist. Das kommt mir etwas komisch/unsauber/hacky vor. Würde dir etwas besseres einfallen? (Anm.: Ich vermute das funktioniert grundsätzlich auch für nicht-pixelbasierte observationen, aber dann müsste man vermutlich noch ein paar transformationen machen. )

Frage 2: Mir ist ein nebeneffekt von dem confreading #41 aufgefallen. Im Moment ist das so, dass wenn man eine Env hat, zu die es env-attr gibt (z.B. atari, oder memoryreacher), dann muss man zumindest immer ein dummy-block in den json einbauen wo zumindest der Type gesetzt ist (siehe z.B. procgen.json in https://github.com/neuroevolution-ai/NeuroEvolution-CTRNN_new/pull/46/commits/55072c53d04e95147140614b3263dd17ad814ec9#). Ist das ein Problem?

(Ich denke das ist erstmal nicht so schlimm. Ein Quickfix wäre möglich, in dem man in EnvHandler.make_env() das assert isinstance nur macht, wenn environment_attributes is not None. Eine saubere Lösung würde den Type im json nicht erfordern, da der ja implizit durch die env gesetzt wird. Aber mir fällt kein Weg ein das zu machen, ohne das die Komplexität in die Höhe schnellt. In diesem Fall würde ich einschätzen, dass der Schaden durch die Komplexität höher ist als der Nutzen durch die vereinfachte config.json. Daher würde ich das einfach mal so lassen)

pdeubel commented 3 years ago

Zu Frage 1: Zur Render Methode würde ich sagen dass passt schon, wir machen bei der env.render() ja genau das Gleiche mit OpenCV. Man könnte allerdings überlegen die beiden Methoden irgendwie zusammenzuführen damit man nicht zweimal diesen Code hat:

        cv2.imshow("ProcGen Agent", frame)
        cv2.waitKey(1)

Vielleicht in helper.py eine Methode:

def render_rgb_array(rgb_array: np.ndarray, str: window_title):
    cv2.imshow(window_title, rgb_array)
    cv2.waitKey(1)

und die kann dann aufgerufen werden mit den entsprechenden Parametern.

Zur Frage 2: Man muss halt immer wissen welchen Type man für die jeweilige Environment braucht. Wenn man das weiß ist es im Prinzip nicht so schlimm, daher würde ich das auch so lassen. Alles andere würde komplizierter werden als nötig denke ich.

bjuergens commented 3 years ago

habe 2 experimente ausgeführt um dask und mp auf procgen zu vergleichen. Beide beginnen bei dem checkpoint data/2020-12-29_00-15-26/checkpoint.pkl

dask_vs_mp.txt

temp.json.txt

Beobachtungen:

pdeubel commented 3 years ago

in der zweiten generation divergieren beiden läufe

Ja wie gesagt das ist gefixt in einem Commit von #45. Der PR ist noch nicht gemerged weil noch nicht gefixt ist warum bei verschiedenen Python Versionen unterschiedliche Ergebnisse rauskommen. Falls das erstmal egal ist kann #45 auch gemerged werden

bjuergens commented 3 years ago

Falls das erstmal egal ist

das mit den seeds und der konsistenz ist alles sowieso ziemlich wackelig. Da werden wir niemals 100%ig stabilität haben. Das hängt von zu vielen Faktoren ab, um das sauber zu machen

pdeubel commented 3 years ago

45 ist gemerged und für #50 hab ich einen Fix hier gepushed -> Dass die beiden Läufe divergieren sollte/könnte gefixt sein