search

neuroevolution-ai / NeuroEvolution-CTRNN_new

MIT License
3 stars 3 forks source link

catch up to old repo #22

Closed bjuergens closed 3 years ago

bjuergens commented 4 years ago

When the new repo is as fast as the old one, we can archive the old one.

This is a meta issue to keep track of this goal.

related issues: #16 #23

bjuergens commented 4 years ago

comment from 2020-09-11 on slack:

Zur Info, ich mache gerade ein paar Performance tests (auf meinem Laptop) zwischen dem alten und dem neuen Repo. Im alten repo komme ich mit dieser Konfig auf 6385s:

{
    "environment": "Hopper-v2", 
    "neural_network_type": "CTRNN", 
    "trainer_type": "CMA_ES", 
    "use_original_cma_trainer": true, 
    "MU_LAMBDA_parameters": 
    {
        "mu": 0.5, 
        "lambda": 1.0, 
        "include_parents_in_next_generation": true, 
        "initial_gene_range": 10, 
        "mutation_Gaussian_sigma_1": 5.0, 
        "mutation_Gaussian_indpb_1": 0.1, 
        "mutation_Gaussian_sigma_2": 5.0, 
        "mutation_Gaussian_indpb_2": 0.01, 
        "mutation_Gaussian_sigma_3": 0.5, 
        "mutation_Gaussian_indpb_3": 0.01, 
        "mutation_Gaussian_dynamic_prob": true, 
        "mutation_Gaussian_sigma_base": 3, 
        "mutation_Gaussian_sigma_factor": 15, 
        "mutation_Gaussian_indpb_base": 3, 
        "mutation_Gaussian_indpb_factor": 15, 
        "elitist_ratio": 0.0, 
        "tournsize": 3, 
        "mutpb": 0.9, 
        "mate": "cxUniform", 
        "mate_indpb_1": 0.5, 
        "mate_indpb_2": 0.1}, 
        "random_seed_for_environment": 0, 
        "keep_env_seed_fixed_during_generation": true, 
        "number_neurons": 10, 
        "parameter_perturbations": 0.0, 
        "delta_t": 0.05, 
        "optimize_state_boundaries": false, 
        "clipping_range_max": 1.0, 
        "clipping_range_min": -1.0, 
        "optimize_y0": true, 
        "set_principle_diagonal_elements_of_W_negative": true, 
        "population_size": 200, 
        "number_generations": 1500, 
        "sigma": 1.0, 
        "number_fitness_runs": 1, 
        "number_neurons_layer1": 32, 
        "number_neurons_layer2": 16, 
        "use_biases": false, 
        "indirect_encoding": false, 
        "cppn_hidden_size1": 8, 
        "cppn_hidden_size2": 4
    }
}

Im neuen repo komme ich mit der adäquaten config auf 7390s: (edited) 


{
    "environment": "Hopper-v2",
    "random_seed": 0, 
    "number_generations": 1500, 
    "optimizer": 
    {
        "type": "CMA_ES", 
        "population_size": 200, 
        "sigma": 1.0, 
        "checkpoint_frequency": 10
    }, 
    "brain": 
    {
        "type": "CTRNN", 
        "number_neurons": 10, 
        "delta_t": 0.05, 
        "normalize_input": false, 
        "normalize_input_target": 2, 
        "optimize_state_boundaries": "fixed", 
        "clipping_range_max": 1.0, 
        "clipping_range_min": -1.0, 
        "optimize_y0": true, 
        "set_principle_diagonal_elements_of_W_negative": true, 
        "parameter_perturbations": 0.0, 
        "w_mask": "dense", 
        "w_mask_param": 4, 
        "v_mask": "dense", 
        "v_mask_param": 4, 
        "t_mask": "dense", 
        "t_mask_param": 4
    }, 
    "episode_runner": 
    {
        "type": "Standard", 
        "number_fitness_runs": 1, 
        "reuse_env": true, 
        "keep_env_seed_fixed_during_generation": false
    }
}```
pdeubel commented 4 years ago

Also ich hab da jetzt mal bisschen reingeschaut und schreib mal meinen Zwischenstand. Folgende Gründe könnten daran liegen:

Erstens

In algorithms.py wird sowohl in eaMuPlusLambda() als auch in eaGenerateUpdate() die folgende Schleife aufgerufen:

for gen in range(toolbox.initial_generation, ngen + 1)

Das hat zur Folge dass immer eine Generation zu viel durchgeführt wird. Beispiel: Man gibt in der config number_generations=3 an, dann wird mit Generation 0 begonnen und das Training endet mit Generation 3, also insgesamt 4. Bedeutet beim Performance Vergleich wird zunächst mal immer eine Generation mehr miteinberechnet.

Vorbemerkung für Zweitens und Drittens

Ich hab vorher ein paar Experimente gemacht um grob abzustecken wo die Performance Unterschiede sind. Dazu hab ich erstmal die Ant-v2 und den Seed 0 festgelegt. Außerdem wird zwar in jedem Schritt der Output des Brains berechnet auf die Umgebung wird allerdings immer nur ein np.zeros(8) Array als Aktion angewendet. Dadurch gibt es dauerhaft gleiche Aktionen und Episoden womit man die Repos besser vergleichen kann. Beim groben Abstecken hat sich dann rausgestellt dass die step() Funktion des CTRNN im alten Repo schneller ist als im neuen.

Zweitens

Bei meinen Experimenten jetzt hab ich eine Population von 50 und 5 CTRNN Neuronen genommen (damit ich schnell viel vergleichen kann). Das hatte zur Folge dass die CSR Matrizen für V, W und T erheblich die Performanz bremsen. Dazu hab ich die step() Funktion in continuous_time_rnn.py im neuen Repo komplett gekürzt sodass nur noch das Standard CTRNN berechnet wird ohne Schnickschnack, also so wie im alten Repo. Dann hab ich die Zeit gemessen die gebraucht wird für die step() Funktion. In den Plots sieht man jetzt auf der x-Achse die Messungen (eine Messung ist eine Ausführung der Step Funktion) und auf der y-Achse die gebrauchte Zeit in Sekunden.

(Beachte dass e-05 am Ende mancher Werte)

Mit CSR Matrizen:

Median Old 3.838539123535156e-05 | Median New 0.0001678466796875
Std Old 6.713789634623939e-06 | Std New 4.3272723182277065e-05
Max Old 0.00013899803161621094 | Max New 0.0010554790496826172
Min Old 2.384185791015625e-05 | Min New 0.00010442733764648438

4-Worker-WithCSR

Ohne CSR Matrizen:

Median Old 3.838539123535156e-05 | Median New 5.3882598876953125e-05
Std Old 6.713789634623939e-06 | Std New 4.130386520750725e-05
Max Old 0.00013899803161621094 | Max New 0.006349325180053711
Min Old 2.384185791015625e-05 | Min New 3.337860107421875e-05

4-Worker-NoCSR

(Das mag jetzt nach nichts aussehen wenn man die Sekunden alleine betrachtet, aber anhand der Anzahl der auf der x-Achse gezeigten Werte sieht man wie das nach oben skaliert, denn das waren lediglich 3 Generationen mit einer Population von 50.) Das Problem mit den CSR Matrizen ist aber denke ich bekannt und ist ja so auch mehr oder weniger erwartet. Vielleicht sollte man nur irgendwie einbauen dass sie nicht immer genutzt werden, damit so ein Verhalten nicht auftritt. Könnte aber auch sein dass das bei schon leicht größeren Matrizen verschwindet.

Drittens

Ich hab lokal Probleme mit Dask. Wenn ich wie voreingestellt n_workers=multiprocessing.cpu_count() in dask_handler.py Zeile 75 lasse, dann bekomme ich eine schlechtere Performanz als z.B. bei 4 Workern:

8 Worker:

Median Old 3.838539123535156e-05 | Median New 0.00010466575622558594
Std Old 6.713789634623939e-06 | Std New 0.0002212902189317166
Max Old 0.00013899803161621094 | Max New 0.015468835830688477
Min Old 2.384185791015625e-05 | Min New 3.5762786865234375e-05

8-Worker

4 Worker:

Median Old 3.838539123535156e-05 | Median New 5.3882598876953125e-05
Std Old 6.713789634623939e-06 | Std New 3.812514721184107e-05
Max Old 0.00013899803161621094 | Max New 0.0043163299560546875
Min Old 2.384185791015625e-05 | Min New 3.314018249511719e-05

4-Worker

In der Dask Documentation wird auf sowas hingewiesen und darauf verwiesen dass bei lokalen Situationen (also kein verteiltes Rechnen über mehrere Computer) andere Lösungen eventuell mehr Sinn machen, z.B. Dask (ohne Distributed), multiprocessing etc.

Wenn wir sagen wir brauchen kein verteiltes Rechnen über mehrere Systeme (ist so glaube ich nicht geplant) könnten wir da was ändern

pdeubel commented 4 years ago

Ich würde vorschlagen offensichtlich erstmal 1. zu fixen (immer eine Genration zu viel), 2. erstmal zu besprechen und bei 3. würde ich Dask ohne distributed testen, d.h. die Funktionen des Frameworks die lokal auf einem Rechner funktionieren und nich das distributed Paket

bjuergens commented 4 years ago

CSR Matrizen sollten das gleich interface haben wie numpy-matrizen. Ich vermute man kann das so implementieren, dass die beiden modis sich nur in der implementierung unterscheiden, sodass insb. in der step funktion keine fallunterscheidung zwischen den beiden mehr gemacht werden muss

Es sollte noch geschaut werden, ob der slow down bei CSR Matrizen von der initialisierung kommt, oder aus der step funktion.

Da das interface für alle sparse-matrizen gleich ist, kann man auch sehr einfach andere spars matrizen verwenden. Vielleicht sind andere ja schneller.

Dask ohne distributed testen

kann ich machen

bjuergens commented 4 years ago

könntest du hier noch schnell das script reinposten mit dem du die daten gemessen und die diagramme erstellt hast

pdeubel commented 4 years ago

Es sollte noch geschaut werden, ob der slow down bei CSR Matrizen von der initialisierung kommt, oder aus der step funktion.

Genau das hab ich ja gemacht. Ich hab immer die step Funktion gemessen, einmal wenn CSR Matrizen benutzt werden und einmal wenn np.ndarrays genutzt werden (Dazu hab ich vor der Zeitmessung die CSR Matrizen mit .toarray() konvertiert)

kann ich machen

Das hatte ich eigentlich vor :D kam im Text nicht so rüber. Wäre denke ich besser da ich da jetzt schon Zeit investiert habe und Vergleichswerte schon hab

pdeubel commented 4 years ago

könntest du hier noch schnell das script reinposten mit dem du die daten gemessen und die diagramme erstellt hast

In beiden Repos ist jetzt jeweils ein Branch compare-performance und im alten Repo ist zusätzlich ein Skript compare-times.py mit dem der Plot erstellt wird.

pdeubel commented 4 years ago

Dask ohne distributed hab ich jetzt noch nicht benutzt, allerdings hab ich Dask einfach mal auskommentiert und einen multiprocessing.Pool benutzt. Das heißt in algorithm.py wird anstatt 

results = toolbox.map(toolbox.evaluate, brain_genomes, seeds_for_evaluation)
with mp.Pool(processes=4) as pool:
    results = pool.starmap(toolbox.evaluate, zip(brain_genomes, seeds_for_evaluation))

ausgeführt.

Ich hatte auch noch einen Dreher in meiner Config sodass im neuen Repo das Individuum jeweils kleiner war. Ich hab den Code zum Testen so angepasst, dass keine CSR Matrizen mehr erstellt werden sondern nur NumPy Arrays, und dass die Individuen gleich groß sind.

Mit Multiprocessing:

Mean Old 3.62574036916097e-05 | Mean New 3.252369085947672e-05
Median Old 3.814697265625e-05 | Median New 2.8848648071289062e-05
Std Old 6.9756152372337835e-06 | Std New 7.529450611892853e-06
Max Old 0.00017642974853515625 | Max New 0.000278472900390625
Min Old 2.384185791015625e-05 | Min New 2.193450927734375e-05

4-Worker-Multiprocessing

Mit Dask:

Mean Old 3.62574036916097e-05 | Mean New 4.722439130147298e-05
Median Old 3.814697265625e-05 | Median New 3.910064697265625e-05
Std Old 6.9756152372337835e-06 | Std New 3.478705054706305e-05
Max Old 0.00017642974853515625 | Max New 0.0011188983917236328
Min Old 2.384185791015625e-05 | Min New 2.384185791015625e-05

4-Worker-KeinMultiprocessing(Dask)

pdeubel commented 4 years ago

Ich hab noch versucht ray zum Laufen zu bringen, hat aber leider nocht nicht geklappt. Ich glaube das liegt daran wie ray serialisiert und das muss man mit DEAP zusammenbekommen. Sollte das aber gehen und die Performanz stimmt fände ich ray nicht schlecht. Da kann man gegebenenfalls auch Cluster nutzen falls man das doch möchte.

Ich würde vorschlagen das wir das am Montag nochmal besprechen was für eine Lösung Sinn macht

  1. Dask Distributed entfernen und nur Dask lokal nehmen. Hier müsste man wahrscheinlich auf DataFrames umstellen
  2. Multiprocessing -> simpel allerdings müsste man gucken wie PyTorch da mitspielt (bei TensorFlow 1 hatte ich da mal Probleme, vielleicht ist es bei PyTorch ähnlich (?))
  3. Ray -> Ist von den DEAP Entwicklern auch als möglicher SCOOP Nachfolger gelistet. Hier wird auch eine mögliche Integration von Ray und DEAP vorgestellt (inoffiziell)
bjuergens commented 4 years ago

Hast Du "Prozesse in Dask" verglichen mit "Threads in anderen Frameworks" oder hast Du in allen Fällen Prozesse verwendet?

Reminder: Der Hauptgrund, warum Dask genommen wurde, ist dass der atari emulator nicht thread safe ist. (Bin mir nicht sicher, ob proc gen auch nicht thread safe ist)

Sehe ich es richtig, dass in den Diagrammen nur die Zeit für die Step funktion berücksichtigt wird, aber nicht die Zeit um den Job zu schedulen und an den worker zu schicken?

Gibt es einen Grund, warum Du die mp-map methode direkt in algorithm.py eingebaut hast anstatt z.B. hier?

Eine Alternative zu Dask sollte folgendes mitbringen:

Und ich vermute, dass #18 auf Dask-Dataframes angewiesen sein wird. Ansonsten müssen wir ein uns ein System überlegen, um genome effizient direkt zwischen den Workern zu verschicken ohne dass der Master zum bottleneck wird.

Wenn wir uns entscheiden müssen zwischen "einfacheres entwickeln" und "mehr effizienz", dann sollten wir auf der Seite der einfacheren Entwicklung bleiben. Wir haben jetzt schon zu wenig man power. Und wenn uns selbst Steine in den Weg legen, dann wird das mit der Manpower nur noch schlimmer.

Da alle multiprozessing-frameworks eine map methode mitbringen, und diese das einzige interface ist, was wir tatsächlich brauchen, möchte ich folgenden vorschlag machen:

Wir bennen den bestehenden parameter "use_worker_processes" (boolean) um zu "mutliprozessing_framework" (string), und geben dem eine option für jedes frame work, was wir benutzen wollen.

Das kann man implementieren in dem man hier 5 zeilen code anpasst.

Das hat den vorteil, dass man auf ein schnelles frame-work mit threads setzen kann, wenn man hyperparametersuche macht, und gleichzeitig kann man ein langsames dask benutzen, wenn man atari spiele lernen will, oder wenn man performance probleme untersucht. Und man kann (wie bisher auch) multiprozessing komplett abschalten, wenn man z.B. exaktes profiling machen will.

pdeubel commented 4 years ago

Hast Du "Prozesse in Dask" verglichen mit "Threads in anderen Frameworks" oder hast Du in allen Fällen Prozesse verwendet?

In allen Fällen hab ich Prozesse verwendet

Sehe ich es richtig, dass in den Diagrammen nur die Zeit für die Step funktion berücksichtigt wird, aber nicht die Zeit um den Job zu schedulen und an den worker zu schicken?

Ja genau wenn man wechselt müsste man das noch zusätzlich messen. Ich hab hier mehr oder weniger Zwischenergebnisse gepostet und da ist mir eben aufgefallen dass allein in der step Funktion diese Zeitunterschiede auftreten wenn man vom einen zum Anderen Scheduler wechselt (Scoop (altes Repo) vs Dask vs multiprocessing).

Gibt es einen Grund, warum Du die mp-map methode direkt in algorithm.py eingebaut hast anstatt z.B. hier?

Ne ich wollte das eigentlich nur schnell testen ob das überhaupt geht

pdeubel commented 4 years ago

Bei ray wäre das glaube ich nicht so einfach da dort keine einfache map Methode zur Verfügung gestellt wird. Ansonsten würde es alle Dinge erfüllen (hat auch ein Dashboard das mitgeliefert wird).

Am liebsten wäre mir wir hätten nur eine Lösung die lokal und im Cluster effizient ist (falls wir Cluster überhaupt brauchen). Können wir ja am Montag besprechen

bjuergens commented 4 years ago

verträgt sich ray gut mit pycharm? Also kann man z.B. einen breakpoint in der step funktion setzen, und der pycharm debugger springt dann da rein?

bjuergens commented 3 years ago

ergebnisse von telco

zu punkt 2:

zu punkt 3:

Wenn wir damit fertig sind, macht daniel einen weiteren globalen vergleich zwischen den repos

hier weiterlesen: https://github.com/ray-project/ray/issues/642#issuecomment-306705096

pdeubel commented 3 years ago

Ich hab angefangen multiprocessing einzubauen: d04148817cf557452c6c15cc78e0595788a71ee4. Muss nochmal drüber gehen bevor in den Master gemerged werden kann.

Hab das jetzt mit dem vorhandenen argparser gemacht, Tap können wir ja nach v01 angehen

bjuergens commented 3 years ago 
if self.number_of_workers <= 1:
        else:
            logging.warning("Continuing with 1 worker")

ich würde hier exception werfen. Offensichtlich hat der User Blödsinn in die Config geschrieben. Das Programm sollte niemals annahmen darüber treffen, was der user eigentlich gemeint hat. (Ausgenommen vielleicht Programme die für den Massenmarkt gedacht sind, aber das ist ein anderes thema)

if self.number_of_workers > system_cpu_count:

gleiches wie oben. Wobei ich hier überlegen würde vllt nur eine warnung zu machen, ohne den parameter zu ändern. Vielleicht hat sich der user ja was dabei gedacht als er den wert so hoch gesetzt hat

class MPHandler:

wäre es sinnvoll ein interface zu machen für MPHandler und DaskHandler? Also z.B. eine abstracte basis klasse von der beide erben: 

class IProcessHandler(abc.ABC):

    def __init__(self, number_of_workers):
        pass

    @abc.abstractmethod
    def map(self, func, *iterable):
        pass

    def cleanup(self):
        pass

    def init_worker_env(self, *nargs, **kwargs):
        pass

dann könnte man sich alle Stellen sparen wo steht if self.parallel_framework ==XYZ

(wobei ich mich gerade frage, warum der DaskHandler überhaupt eine init_workers_with_env-Methode braucht, und das nicht schon im constructor macht.)

pdeubel commented 3 years ago

Ich hab die Warnings raus und in beiden Fälle einen RuntimeError geworfen. Ist denke ich auch im zweiten Fall besser damit der User das gleich merkt, er könnte es leicht im Log überlesen und später denken er hätte 1000 Worker genutzt obwohl es nur 8 oder so waren.

Zu dem Interface würde ich noch warten, kann ja sein dass wir doch nur ray (oder was anderes) benutzen, dann würde das sowieso wieder rausfliegen

pdeubel commented 3 years ago

Zeitmessungen Erstellung von Environments und Brain

Ein Problem von einem multiprocessing.Pool ist, dass man im Moment keine Environments reusen kann (bei Dask geht das: jeder Worker hat eine Environment für seine gesamte Lebensdauer). Theoretisch kann man auch für jeden Aufruf von eval_fitness eine neue Environment erstellen. Hier eine Zeitmessung dafür, benutzt wurde Reacher-v2 mit der Default Config:

time_creation = timeit.timeit(lambda: self.env_handler.make_env(self.env_id), number=10000)

Ergebnis in Sekunden bei 8 Workern:

102.1730536770001
102.9757395380002
103.80225929700009
103.74805471000036
104.5384372489998
103.41718992299957
102.97499266799969
103.72944322400053

Das heißt um 10000 Environments zu erstellen bracht man mindestens 102 Sekunden, oder ca. 0.0102s pro Umgebung. Wenn man z.B. ein Experiment mit 1000 Generationen und einer Populationsgröße von 200 startet wäre der Aufwand für das Erstellen der envs: 1000 * 200 * 0.0102 = 2040, also ca. 0.5h.

Das Gleiche kann man übrigens auch auf das Brain übertragen:

time_brain_creation = timeit.timeit(
                lambda: self.brain_class(self.input_space, self.output_space, individual, self.brain_config),
                number=10000)

Ergebnis in Sekunden bei 8 Workern (sind wohl welche abgeschmiert beim messen):

61.70068908699977
65.07532928300043
62.44906525799979
64.45642831199984
64.98696891800046
67.62371881099989

Die Dauer ist also eher geringer als bei der Environment, allerdings wird das Brain in jeder Runde neu erstellt, das heißt wenn config.number_fitness_runs z.B. auf 20 gesetzt wird, wird aus den 1000 * 200, 1000 * 200 * 20. Konkret: 1000 * 200 * 20 * 0.0062 = 24800, bzw. ca. 6.9h.

Insgesamt also 6.9h + 0.5h = 7.4h unnötiger Overhead bei dem konkreten Beispiel jetzt.

Das wurde jetzt nur für Reacher-v2 getestet mit der Standard config, das heißt das Hinzufügen oder weglassen von gym.Wrappern wird da sicherlich auch mit reinspielen.

Vorschlag

Mein Vorschlag wäre, unabhängig von dem parallel processing framework, pro Worker einen EpisodeRunner zu erstellen, der für die Lifetime des Workers bestehen bleibt. Dort sind als Instanzattribute die Environment und das Brain, dann kann in jedem eval_fitness darauf zugegriffen werden. Für Umgebungen die immer neu erstellt werden müssen (z.B. procgen, wegen des Seeds) kann man dann die _get_env() Methode umschreiben bzw. so lassen sodass dann eine neue erstellt wird.

Die map Funktion würde dann nicht direkt auf ep_runner.eval_fitness zeigen sondern auf die Handler (DaskHandler/MPHandler), die eine eigene map Funktion definieren. Dort wird die Arbeit auf die Worker des Frameworks verteilt.

DaskHandler: Dort wird beim Setup pro Worker ein EpisodeRunner Objekt als Instanzvariable über das _CreatorPlugin erstellt. Ich denke man muss dann in der map Funktion im DaskHandler alle Worker ansprechen, dass sie in ihrem EpRunner eval_fitness ausführen sollen. Vielleicht geht das mit der transition() Methode (Zeile 57)

MPHandler: Im Setup müsste man von Pool auf mehrere Process Objekte wechseln, damit man Zugriff auf sie hat und ihnen einen EpisodeRunner geben kann. In der map Funktion wird dann einfach auf die Processes verteilt.

Des Weiteren bräuchte jedes Brain eine set_individual(individual) Methode mit der das neue Individual gesetzt wird. Ich denke das kann man aber schnell implementieren, da das im Konstruktor ja sowieso schon für jedes Brain gemacht wird.

@bjuergens Müsste man sonst noch irgendwas machen, bzw. wie findest du das?

bjuergens commented 3 years ago

Mein Vorschlag wäre, unabhängig von dem parallel processing framework, pro Worker einen EpisodeRunner zu erstellen, der für die Lifetime des Workers bestehen bleibt

hört sich gut an

Dort sind als Instanzattribute [...] das Brain

warum das brain?

Für Umgebungen die immer neu erstellt werden müssen (z.B. procgen, wegen des Seeds) kann man dann die _get_env() Methode umschreiben bzw. so lassen sodass dann eine neue erstellt wird.

Implementierungsdetails der env sollte nicht in den eprunner. Den workaround für procgen plane ich als wrapper zu implementieren

Die map Funktion würde dann nicht direkt auf ep_runner.eval_fitness zeigen sondern auf die Handler (DaskHandler/MPHandler), die eine eigene map Funktion definieren. Dort wird die Arbeit auf die Worker des Frameworks verteilt.

verstehe ich nicht. (Also überhaupt nicht. Die map methode zeigt doch jetzt auch schon auf den MPHandler/DaskHandler. Und warum soll an der stelle überhaupt was geändert werden?)

Des Weiteren bräuchte jedes Brain eine set_individual(individual)

Verstehe ich auch überhaupt nicht.

Das klingt alles nicht als könnte man das irgendwie sauber implementieren ("sauber" meine ich im Sinne von SOLID und so). Mit dem Vorschlag müssen viele Klassen Details von anderen Klassen kennen, und man braucht viele fallunterscheidungen.

Ich verstehe auch nicht ganz, was der unterschied im worker-life-cycle von MP und Dask ist, und wo die die unterschiede vom handling der envs herkommen.

könnte man dem MP/Dask-Handler eine methode geben, die da heißt get_env(), und der EpRunner ruft das einfach einmal am anfang auf, um sich die env geben zu lassen? (Bei Dask würde die methode dann einfach return worker.env machen, und bei MP etwas analoges). Der nachteil wäre, dann der der EpRunner den Handler kennen muss

Und in der get_env methode wird einfach ein singleton zurückgegeben. Und wenn das objekt in diesem Process noch nicht initialiisert wurde, dann soll das jetzt angelegt werden. (wobei es irgendwie eine warnung geben sollte, wenn das objekt zu oft angelegt wird)

Ich würde sogar vermuten, dass das anlegen von diesem singleton noch nichtmal MP/Dask spezifisch ist. Man kann das vielleicht sogar direkt im EpRunner machen, und kannn dann das Zeug mit dem _CreatorPlugin in Dask einfach rauswerfen.

--> Wäre es möglich in der Klasse EpRunner eine Methode get_env() anzulegen, die ein singleton für die env zurückgibt, und bei bedarf für diesen Prozess anlegt?

Wenn ja, dann kann man das ganze env-handling aus dem multiprocessing-handler einfach direkt rauswerfen, denke ich

pdeubel commented 3 years ago

Wäre vielleicht besser das nochmal in einem Call zu besprechen bis dahin:

Dort sind als Instanzattribute [...] das Brain

warum das brain?

Siehe https://github.com/neuroevolution-ai/NeuroEvolution-CTRNN_new/issues/22#issuecomment-731168235, ab "Das Gleiche kann man übrigens auch auf das Brain übertragen:" Das Brain jedes mal neu zu initialisieren ist auch recht zeitintensiv und ich denke es würde mehr Sinn machen das nur einmal zu machen.

Des Weiteren bräuchte jedes Brain eine set_individual(individual)

Verstehe ich auch überhaupt nicht.

Die neue Methode wäre deshalb nötig, da man dann pro EpisodeRunner Objekt ein Brain Objekt hat und das muss dann natürlich immer die neuen Individuen bekommen. Es kann natürlich sein dass die Zeit um ein neues Brain zu konstruieren nicht wesentlich länger ist als schon ein vorhandenes Brain zu haben auf dem nur das Individuum neu gesetzt wird. Das werde ich noch messen.

Für Umgebungen die immer neu erstellt werden müssen (z.B. procgen, wegen des Seeds) kann man dann die _get_env() Methode umschreiben bzw. so lassen sodass dann eine neue erstellt wird.

Implementierungsdetails der env sollte nicht in den eprunner. Den workaround für procgen plane ich als wrapper zu implementieren

Das habe ich auch gar nicht vor :) der EpisodeRunner hat ja einen EnvHandler als Instanzattribut, der würde benutzt werden. Die _get_env() Methode würde so ähnlich aussehen wie sie jetzt auch gerade ist.

Das klingt alles nicht als könnte man das irgendwie sauber implementieren ("sauber" meine ich im Sinne von SOLID und so). Mit dem Vorschlag müssen viele Klassen Details von anderen Klassen kennen, und man braucht viele fallunterscheidungen.

Denke ich nicht, also das würde sauber gehen: In algorithms.py wird wie vorher die toolbox.map Funktion genutzt. Diese wird zuvor eben auf die map Funktion des DaskHandlers/MPHandlers/RayHandlers etc. gesetzt. Der EpisodeRunner weiß von nichts der wird nur von den Handlern aufgerufen.

Die einzige Fallunterscheidung wäre denke ich in experiment.py wenn das entsprechende Framework ausgewählt wird.

Ich verstehe auch nicht ganz, was der unterschied im worker-life-cycle von MP und Dask ist, und wo die die unterschiede vom handling der envs herkommen.

Der worker-life-cycle ist der Gleiche. Der Unterschied liegt darin, dass man in einem multiprocessing.Pool keinen Zugriff auf die einzelnen Worker direkt hat. Diese werden beim Aufruf erstellt und danach gelöscht. Das heißt man kann denen keine Attribute zuweisen (z.B. wie im aktuellen Stand eine Umgebung, bzw. so wie ich es vorgeschlagen habe wäre es ein EpisodeRunner). multiprocessing hat nicht so WorkerPlugins wie dask. Eventuell könnte man eine Subklasse von multiprocessing.Process erstellen und dort Instanzvariablen für Prozesse haben. Das managen der Prozesse würde dann in MPHandler stattfinden.

Und in der get_env methode wird einfach ein singleton zurückgegeben. Und wenn das objekt in diesem Process noch nicht initialiisert wurde, dann soll das jetzt angelegt werden. (wobei es irgendwie eine warnung geben sollte, wenn das objekt zu oft angelegt wird)

Sowas könnte man auch machen

pdeubel commented 3 years ago

Ergebnis des Calls:

Außerdem: ray testen. Ansatz funktioniert, muss noch besser integriert werden und dann die Zeit gemessen werden

bjuergens commented 3 years ago

ich denke wir sollten die issue hier zumachen, und dafür 1-2 kleineres neue issues mit konkreten TODOs anlegen.

bjuergens commented 3 years ago

I think we are done here. We are using MP as multi-processing handling. And we fixed the transpose-error introduced by me.