Giter Club home page Giter Club logo

Comments (25)

jindrahelcl avatar jindrahelcl commented on May 9, 2024 1

A tomu helperu bych dal nějakej jasnější název, třeba create_or_load, místo initialize.

from neuralmonkey.

jlibovicky avatar jlibovicky commented on May 9, 2024

I described loading configuration in #15. It is just a scenario, how python object should be built. The other thing is generation the run.ini configuration. It now works such that it is given a dictionary of objects and it generates the ini file that would generate exactly the same object structure. This cannot be done with higher order functions unfortunately. The other option would be to generate it directly from the training configuration which wanted to avoid, because in this way, I can ignore the semantics of the configuration work only with only syntactically.

from neuralmonkey.

tomasmcz avatar tomasmcz commented on May 9, 2024

@jlibovicky: You really need to learn to read what you write, half of your last comment (particularly the last sentence) makes no sense to me, apparently because some words are missing (and some are duplicated).

What I wanted to know (and it wasn't clear to me from previous comments) is the fact that you need another config to run the trained model and you generate it from the objects. That is a design decision that I just don't get. Why not generate it from the parsed train configuration (apart from the fact that in the first version there was no such thing)?

from neuralmonkey.

tomasmcz avatar tomasmcz commented on May 9, 2024

Now I learned in #19 that the thing called runner is actually doing the work and it has its specification in the first ini file. Why do we even generate another ini file?

from neuralmonkey.

jlibovicky avatar jlibovicky commented on May 9, 2024

The reason why we generate a new ini file for running the is that the experiments produce files that do not exist when you run the experiment configuration, namely the pickled vocabularies and the model variables. Moreover, there some information specific for the training which does not matter at all while running the model.

from neuralmonkey.

tomasmcz avatar tomasmcz commented on May 9, 2024

@jlibovicky : Again, in an abstract discussion like this one, the words missing in your text make it quite hard to understand.

Files does not exist. So what? I don't see why that would be a reason to generate any configuration from existing objects. I know from the original ini file what vocabularies and other entities should have been created and therefore I know where to find the files.

There is information in the original ini file that will not be used while running the model. So what? Can't you just ignore it?

from neuralmonkey.

jlibovicky avatar jlibovicky commented on May 9, 2024

So then, you would like to have a different semantics of the same file while loading it from training and for running? Like: file "file_name" would mean save the vocabulary here, if you would load it for training and load the vocabulary from here you while loading it in runtime?

It sounds a bit dangerous to me. You would need to hardcode properties of at least some data structures (at least datasets and vocabularies) which means managing their functionality on two different places which is something I would like to avoid.

from neuralmonkey.

tomasmcz avatar tomasmcz commented on May 9, 2024

The semantics is quite clear: the vocabulary is saved here. When you're the one doing the saving, you save it there, when you're doing the loading, you load it from there. I don't see how these two things can ever be independent. There would have to be a situation where you're supposed to put something at A and pick it up from B and between these two actions something else, not mentioned in the configuration, would move the thing from A to B. Which is the dangerous scenario again?

Since everything in the second configuration can be inferred from the firs one, there is no need for the second configuration. And even if there was, it can be crated from the first configuration (after it is parsed), it does not have to be generated from the results of the first configuration.

Maybe the 2-config solution solves some technical problems. But it creates just as many (inability to use higher order functions is just one of them) and it does not make any sense.

from neuralmonkey.

jlibovicky avatar jlibovicky commented on May 9, 2024

Načítání funguje tak, že takový zjednodušeně stavebnicově napsaný Python. Neobsahuje nic, co by vlastně bylo specifické pro tenhle projekt - prostě jenom interpretuje nějak literály, volá to funkce nebo __init__ metody a strká to do nich to, co se tam napíše (proto je mimochodem nesmysl před všechno automaticky připisovat neural monkey). Mapuje se to 1:1 k pythonovému kódu. To mi přijde naproto transparentní a jednoduché a nechal bych to takhle. Chápu, že to Perl/PHP-like zacházení s literály se někomu nemusí pozdávat, mně přijde pro takovouhle skriptovací záležitost docela elegantní.

S tou konfigurací navrhuju ale udělat následující (většina už tady padla):

  1. V modulu config udělat helper metodu, která bude vytvářet dataset. __init__ metoda datasetu potom bude transparentě dostávat jenom svoje jméno a dictionary jednotlivých series. (Až to bude, tak se můžeme dohadovat, jestli má být Dataset třída)
  2. V modulu config udělat helper metodu, která bude dělat vocabulary. Dostane cestu, dataset a id datové řady. Pokud bude existovat soubor, kde je vocabulary uložená, tak jí načte ze souboru, pokud ne vytvoří ji z datasetu a uloží do toho souboru. (Tohle je jediný tricky moment, je potřeba zařídit, aby se ten dataset nevyhodnocoval, když to není potřeba.)
  3. Používat tentýž konfigurák jak pro experiment, tak pro spouštění modelu (ať už dávkově nebo jako web service) Technicky to znamená přestat config.configuration.Configuration, aby netestovala, jestl tam nejsou nějaké argumenty navíc, jenom je prostě nevyhodnocovat.

Tohle podle mě vyřeší skoro všechny problémy, co s tím teď jsou. Nebude problém používat higher-order funkce. Z enkodérů, dekodéru, trainerů a runerů zmizí hodně atributů, tkeré tam teď byly jenom proto, aby se mohly při generování run konfigurace zase uložit, pylint bude mít klidnější spaní.

Pokud více méně souhlasíte s tím, aby to tak bylo, tak mi pište připomínky, já to zítra naprogramuju. Pokud myslíte, že je potřeba ještě diskutovat, diskutujme, ale už to bude programovat někdo z vás dvou.

P.S. Už mě obtěžuje psát anglicky.

from neuralmonkey.

jindrahelcl avatar jindrahelcl commented on May 9, 2024

+1 za angličtinu.. Anglicky se špatně nadává..

from neuralmonkey.

tomasmcz avatar tomasmcz commented on May 9, 2024

Do whatever you think is best, this seems like a reasonable design. If you weren't leaving, I'd like to leave a few more days for discussion, but since we don't have time, this will definitely be better than the thing that's there right now.

Just a few smaller things, feel free to ignore them if you disagree or think it would take too much time:

  • In number 1 and 2 I don't really understand what you want to do, but do it anyway. In number 2 it seems like you want to branch on whether the vocabulary file exist. I think it would be much better to have a config reader (or object generator) that knows whether its supposed to build a training or a running configuration and act accordingly. Which means when you say "run this.ini" and the vocabulary files aren't there, it should complain, not just create them.
  • This isn't about pylint. It's about maintainable code and long-term functionality. And therefore, it's about whether your work in few last months will ever be useful for something.
  • I would still prefer to use ConfigParser for the first stage of the ini parsing. It would shorten the code and raise reasonable exceptions when you make a real syntax error

If you cannot curse in English, the correct solution is to learn to curse in English, not to curse in Czech. I, for one, am grateful for this opportunity to practice my written English (and if you struggle with it and get bothered by it, then you clearly need some practice as well). If you don't want to learn, what the heck are you even doing on a postgradual? (This goes for Python, git, etc. as well.)

from neuralmonkey.

jindrahelcl avatar jindrahelcl commented on May 9, 2024

Correct solution je odjet z čech a naučit se pořádně anglicky tam. A to teď taky dělám. Psát si v týhle situaci anglicky s čechama je přinejmenším zvláštní.

from neuralmonkey.

jindrahelcl avatar jindrahelcl commented on May 9, 2024

Nechci zakládat žádnej novej thread o konfiguraci a nevyhovuje mi google-dokumentovej mini-widget na diskusi. Proto znovuotevírám tohleto s následujícíma nápadama. Nutno poznamenat, že ačkoli mi ty věci, co se chystám napsat přijdou, jako že by byly fajn, vlastně nakonec vůbec fajn bejt nemusej.

Myslím si, že by z designovýho hlediska by bylo nejlepší celou konfiguraci pojmout tak, že si každej objekt svoje konfiguráky bude deklarovat sám, něco ve stylu argparse, ale pro náš neuralmonkey INI soubor. To by mohlo vypadat nějak následovně:

INI soubor:

[frobnicator]
class=omgs.frobnicator
frobnication_size=256
omg_size=31415927
foo_here="/tmp/foos"

Třída frobnicator (a všechny ostatní neuralmonkey konfigurovatelný třídy) by pak uměly něco jako tohle:

@staticmethod
def gather_configuration_args(self, argparser):
  argparser.add_argument(self, "frobnication_size", required=True, type=int)
  argparser.add_argument(self, "omg_size", required=False, type=int)
  argparser.add_argument(self, "foo_here", required=True, type=ArgParse.filetype) #nebo jak to je

def __init__(self, **kwargs):
  self.frobnication_size = kwargs.get("frobnication_size", DEFAULT_FROB_SIZE)

Výhodou tohohle vzoru je, že při parsingu konfigurace by se napřed na každý třídě zavolalo gather_configuration_args, který by dělalo kontrolu argumentů před tím, než by se zavolal konstruktor, takže chyby v konfiguraci by se projevily hned na začátku.

Taky by nám to pomohlo zlikvidovat megakonstruktory. Když se dá [main] do samostatnýho objektu, tak to bude chodit zadarmo rovnou.

Blbý je, že by se rozbily ty helper funkce. Teď mě nenapadá jak tohleto udělat. Ale ani mě, když na to koukám nenapadá, proč to není v konstruktorech těch objektů (ale to už bylo předmětem jiný diskuse)

Taky bych na parsing zkusil použít https://docs.python.org/3/library/configparser.html místo našeho. Přineslo by to mimo jiné spoustu pěknejch vlastností, jako třeba zadarmo interpolaci proměnných nebo možnost odkázat se v manuálu na manuál toho parseru.

Disclaimer: Nenavrhuju, že to takhle máme dělat a uvědomuju si, že je to příliš velká změna v návrhu. Pokud tahle diskuse dospěje do místa, kdy si budu jistej tím, že je to hezčí, tak to udělám sám. Nestane se to, pokud to dospěje do příliš abstraktních témat. Pro abstraktní komentáře proto využijte smajlíky v "pick your reaction" a pokuste se nekomentovat tenhle disclaimer. Ď

from neuralmonkey.

jlibovicky avatar jlibovicky commented on May 9, 2024

ConfigParser klidně, jenom mi přijde, že ohnout ho tak, aby dělal to, co my chceme, je složitější, než napsat to, co tam je teď, navíc (asi) ztratíme možnost osobně trackovat, co se děje na jaké řádce. Ale tohle je důvod, proč se to mně nechtělo dělat. Jestli se ti to chce (nebo Tomášovi) nastudovat a implementovat, jsem jedině pro.

No ty argumenty (jména a přítomnost) se tam kontrolujou už teď. Prostě se to koukne na to, co to má vzít za argumenty a když nesedí, tak to dá rozumnou chybovou hlášku, proč ty argumenty nesedí.

Jako velké omezení bych viděl, že nejenom, že to znemožní používat higher-order functions, ale všechny funkce obecně. Co bychom tím získali by byla nějaká nepovinná kontrola typů, ale ty bylo možné kontrolovat stejně jenom, až by se ty věci exekuovaly. To bych radši dělal tak, že by se kontruktorech a funkcích, o kterých očekáváme, že se budou volat z konfigurace, psaly nějaké explicitní aserce, připravil jsem na to modul checking (a kontrolovaly si to navzájem při pull requestech). [Teď mě tak napadá, jestli bysme si pro takovéhle věci, co jdou těžko dělat automaticky neměli udělat nějaký checklist.]

Věci, které se týkají vyloženě konfigurace v konstruktorech objektů nejsou, protože vytvářet ty objekty z konfigurace není jediný způsob. Dataset nemusím načítat ze souborů, může mi přijít do webové služby například. Proto potřebuje ty 'helper functions,' které nahradí vícenásobné konstruktory. Aspoň nás to nutí dávat do konstruktoru jenom to, co ty objekty skutečně potřebují, obzvlášť, když třeba ten dataset je vlastně algebraický datový typ.

Že by nebyl žádný main, jenom callable a run a train configuration objekty, to se mi líbí.

from neuralmonkey.

jindrahelcl avatar jindrahelcl commented on May 9, 2024

Přišel jsem teď na use-case, kterej ukazuje, proč neni ideální mít initialize_vocabulary pro načítání i ukládání slovníku dohromady.

Situace: Mám doběhnutej experiment, kterej uložil svuj vocabulary někam do souboru. Chci ho pustit dotrénovat se stejným slovníkem, ale s jinými daty (přesněji podmnožinou původních dat). Normální chování train skriptu je, že ukládá slovník. Teď ale stejná konfigurace bude slovník načítat.

Problém: Pokud bych to nedělal schválně a chtěl trénovat stejný model od začátku na jiných datech (např. jiný jazykový pár při překladu) jen si nevšimnul, že už stejně pojmenovaný soubor se slovníkem existuje, místo vygenerování a uložení novýho slovníku z nových dat by se mi načetl slovník z úplně jiného jazyka. Za těchhle okolností by to celý běželo bez jedinýho varování jako na drátkách, akorát bych po pár hodinách (v závislosti na podobnosti těch jazyků i za delší dobu) viděl, že to generuje samý nebo jiný blbosti.

Tenhle scénář mi přijde velmi pravděpodobnej, protože když mám konfigurák a chci jen pozměnit vstupní data, tak ten původní konfigurák zkopíruju a ručně oedituju. Kdyby tam bylo explicitně, jestli chci vocabulary načíst nebo uložit, tak by to spadlo na (ne)existujícím souboru.

Návrh řešení: Funkce initialize_vocabulary je stejně jeden if, takže navrhuju rozseknout jí na vocabulary_from_pickle a vocabulary_from_data. Na generování spouštějícího configu bych se prozatim vykašlal a nechal to na každym, ať si to překopíruje. Nebo nevim. Možná by šlo to udělat tak, že trainer by používal slovník vocabulary_from_data a runner by měl slovník vocabulary_from_pickle a ukazovalo by to na stejný data. Při načítání by se tam dala obezlička, která by vocabulary_from_pickle řikala, jestli to má fakt načítat ze souboru, anebo jestli má použít objekt, kterej teprv uloženej bude.

from neuralmonkey.

jindrahelcl avatar jindrahelcl commented on May 9, 2024

Related pull request #70

from neuralmonkey.

jlibovicky avatar jlibovicky commented on May 9, 2024

No... já bych si myslel, že si ty vocabulary budu typicky ukládat vždycky do té složky s experimentem, takže tohle nebude hrozit. Ten if je tam právě proto, aby se mohla stejná konfigurace používat pro trénovaní i pro pouštění a vlastně přepíná právě mezi tady těma dvěma funkcemi, které najdeš ve vocabulary buď jako funkce nebo statické metody.

from neuralmonkey.

jindrahelcl avatar jindrahelcl commented on May 9, 2024

No ale právě když mám dva experimenty (např. alpha a beta), a chci načíst slovník (i třeba variables) z experimentu alpha a pokračovat v trénování na jiných datech, tak v konfiguráku od beta musim mít cesty k těm souborům do alpha adresáře (pokud to nechci kopírovat).

Kdybych mohl použít config pro načtení a zároveň config pro uložení, mohl bych v betě mít jednak ukazatel do adresáře alphy a jednak ukazatel kam by se pak nahrál doopravdy použitej slovník pro betu (za předpokladu že mi třeba beta ten slovník trochu pozmění, abychom se nehádali o tom, že jsem to měl rovnou zkopírovat).

Taky z pohledu vnitřní logiky toho programu mi přijde, že o načítání a ukládání by se měly starat jiný komponenty. Při trénování můžu potřebovat obojí, ale při pouštění bych slovník ukládat neměl.

from neuralmonkey.

jindrahelcl avatar jindrahelcl commented on May 9, 2024

Je potřeba vyrefaktorovat a izolovat model a všechny jeho náležitosti (včetně možnosti načítání proměnných/slovníky a třeba i preprocessingu) od všeho ostatního (trainer, data, ukládání proměnných a slovníku)

from neuralmonkey.

jlibovicky avatar jlibovicky commented on May 9, 2024

Nejsou to překombinovaný scénáře? Pokud chceš načíst slovník i variables, tak určitě nechceš ten slovník zároveň měnit.

Navíc jsem myslel, že když jsem se dohodli, že bude pro uživatele nejpřehlednější mít jeden konfigurák, tak jsme museli rovnou počítat s tím, že tadyta neprůhlednost v těch vocabulary je daň, kterou za to zaplatíme. (Možná jsem to měl vykřikovat explicitněji, pokud jsem byl jediný, kdo si to myslel.)

Kdyby se generoval zvlášť pro konfigurák pro spuštění, tak v trénovacím konfiguráku alpha bude from_dataset, v jeho spouštěcím bude from_pickle a v trénovacícm beta bude už jenom from_pickle.

Pokud se nechceme vracet ke dvěma konfigurákům (což bych nerad, protože to komplikuje dost věcí), tak bych navrhoval asi zavést povinné from_pickle a slovník vytvářet nějakým skriptem dopředu. Ale klidně bych nechal takhle, mně to nepřipadá nebezpečné.

from neuralmonkey.

jindrahelcl avatar jindrahelcl commented on May 9, 2024

Překombinovaný to spíš je. Jeden konfigurák je rozhodně správnej krok, na tom nic měnit nechci. Ani nechci žádný konfiguráky generovat.

Pomalu v hlavě dokonvergovávám k tomu, jak to je teď udělaný. Možná jediný, co bych dodělal, je vyžadovat vocabulary pickle, když se to pouští z run skriptu.

from neuralmonkey.

jindrahelcl avatar jindrahelcl commented on May 9, 2024

Novej případ, kdy se implicitně udělá chyba.

Vemeš konfigurák od jednou proběhýho experimentu, změníš vocabulary size a pustíš znova. Čekal bys, že to pojede s tvojí novou vocabulary size, ale on se načte ten starej slovník a ty nevíš nic.

from neuralmonkey.

jlibovicky avatar jlibovicky commented on May 9, 2024

No řek bych, že v tom případě si máš ty vocabulary ukládat do složky s experimentem (opatřený timestampem). Ale jestli si myslíš, že je častej případ, že chceš mít někde vocabulary stranou, tak bych asi vynucoval přípravu vocabulary zváštním skriptem a do hlavního konfiguráku dával jenom zapicklovanou vocabulary.

from neuralmonkey.

jindrahelcl avatar jindrahelcl commented on May 9, 2024

good point, davat do ke kazdymu experimentu s timestampem zvlast me nenapadlo.

from neuralmonkey.

jindrahelcl avatar jindrahelcl commented on May 9, 2024

Closing. Mam pocit že většina věcí, co se tady řešily, už je nějak vyřešená. Vracet se k týhle starý diskusi už asi nebudem, přišlo by mi vhodnější případně otevřít novou.

from neuralmonkey.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.