Integrated kmers and adjusted reproducibility #49
Integrated kmers and adjusted reproducibility #49MiriamBalzer wants to merge 14 commits intodevelopmentfrom
Conversation
…nistic logic for stacked approach -> reproducable results
…h different python versions
Julian-W98
left a comment
Julian-W98
left a comment
There was a problem hiding this comment.
Ich habe einige Unstimmigkeiten bezüglich constanten/variablennutzung gefunden. Auch eine große Code Dublikation in preprocessing.py ist mir aufgefallen.
Um zu überprüfen welcher Teil deines Codes für deterministisches Verhalten sorgt habe ich deine Änderungen einzeln getestet. Tatsächlich scheinen nur die beiden random_state=42 in classic_rf.py und stacked_rf.py den Unterschied zu machen. Zumindest hatte ich so mit den gleichen Daten im Modus TRAIN_TEST auf zwei verschiedenen Nodes das selbe Ergebniss. Alle weiteren Änderungen in stacked_rf.py und utils.py sollten daher aus meiner Sicht nicht vorgenommen werden weil sie nur unnötig die Komplexität erhöhen.
Sonst sind mir nur Kleinigkeiten aufgefallen
workflow/config.py
Outdated
| MIN_SAMPLE_NUMBER = 15 | ||
| LOGGING_LEVEL = logging.INFO | ||
| KMERE = False | ||
| W2V_MODE = W2VMode.TUNE_W2V |
There was a problem hiding this comment.
Warum gibt es jetzt den W2VMode und RETRAIN_W2V?
Wäre davon ausgegangen, dass W2VMode.TRAIN_W2V das gleich ist wie RETRAIN_W2V = True
Vielleicht die Benennung hier etwas anpassen um das Unterscheidbar zu machen wenn es wirklich beides braucht
There was a problem hiding this comment.
Ja, das ist dem Wachstum der implementation geschuldet. dadurch, dass ich nachträglich die Möglichkeit hinzugefügt habe das trainierte Modell zu speichern und wieder abzurufen. Ich schau mal, ob ich das etwas eindeutiger und weniger repetativ implementiert bekomme :)
| TUNE_HYPERPARAMETER = "tune_hyperparameter" | ||
|
|
||
|
|
||
| class W2VMode(Enum): |
There was a problem hiding this comment.
Ist es gewollte das man jetzt W2V auf Train und den Execution Mode auf Predict stellen kann?
Hätte jetzt eher vermutet, dass wenn k-mere auf an gestellt wird und Execution Mode auf training steht der V2W auch mittrainiert wird.
There was a problem hiding this comment.
theoretisch kann W2V immer mit laufen, sobald das preprozessing angeschmissen wird, da es ja eben ein vorverarbeitungsschritt ist. Natürlich ist das aber nicht immer sinnvoll 😅 allerdings auch nicht wirklich schlimm. ich schau es mir aber nochmal an
…m, normal random forest deleted, changes in utils.py undone, minor changes according to PR comments
2 participants
No description provided.