ufal / neuralmonkey

An open-source tool for sequence learning in NLP built on TensorFlow.
BSD 3-Clause "New" or "Revised" License
410 stars 104 forks source link

BERT: data preparation script, tests #810

Closed varisd closed 5 years ago

varisd commented 5 years ago

Also:

jlibovicky commented 5 years ago

Ještě neprochází test, protože konfigurář bert.ini má nepoužívané sekce.

jlibovicky commented 5 years ago

@varisd upravil jsem ten konfigurář, aby používal všechny sekce. Pokud si myslíš, že je to dobře, tak merguj.

jindrahelcl commented 5 years ago

Nezkoušel jsem to, ale tf.where vždycky propouštělo komputaci do všech větví. tf.cond taky, ale to už možná zafixovali.

jlibovicky commented 5 years ago

Přidal jsem test a zjistil jsem dvě věci:

jlibovicky commented 5 years ago

Takže navrhuju smazat celej ten tenzor output_mask

varisd commented 5 years ago

Ad output mask: ten opravdu tedka nepojede z jednoho prosteho duvodu: Ta lambda funkce, co se vola z tf.cond je spravne pouzita, ovsem ten train_inputs Placeholder se vytvari uz behem ``register_inputs''. Prekvapuje me tento design choice, protoze register_inputs registruje uplne vsechny mozne placeholdery (pro pozadovane series; definovane v ruznych objektech v konfigu) bez ohledu na to, jestli se dane series vubec nachazeji v poskytnutych datasetech.

Dalsi vec je, ze pokud dana series (jej placeholder) potrebna pouze pro trenink, nebo pouze pro vypocet nekterych casti grafu, tak by take jeji tvorba mela byt lazy (kdyz uz jste si zvolili tento design).

jlibovicky commented 5 years ago

To neni žádnej design choice, to je provizorní věc na cestě k tf.Datasetu, kterou je tam jenom proto, aby to zatím fungovalo, než Jindra v listopadu dodělá refaktor s tf.Datasetem. Takže tady na ten debilní mechanismus bych se neohlížel, protože tam snad brzy nebude. Místo těch placeholderů tam přicestuje tf.Dataset, kterej tam prostě ten tenzor mít nebude.

varisd commented 5 years ago

Ok, nechal bych to bud on-hold do tf.Datasetu (tam uz to snad pujde i s tim tf.cond) a nebo to zamergoval s poznamkou, ze tam v budoucnu chceme mit prepinani masek