Kyle Gorman

Results 217 comments of Kyle Gorman
trafficstars

I created issue #142 to document the plan here.

I'm not a merge vs. rebase N*zi but the way the PR looks when I review is plenty clear as is.

> Does this mean we would call it like `index.index` (consider that in dataset.py wew currently have e.g. `self.index.vocab_map`)? You could make it so `self.index` could so be called, i.e.,...

Why not just make it the case that the target vocab are the first or last N elements of the vocabulary and then keep track of what N is? Then,...

> I have tested for Attentive LSTM and pg LSTM, and it solves the indexing issue Michael has. When I come back to this tomorrow I also want to set...

> Look what we did for `--log_wandb` and `--no_log_wandb` [here](https://github.com/CUNY-CL/yoyodyne/blob/a8340043e9834310b51f71bfe2f5470a345ae5f7/yoyodyne/train.py#L317). Nevermind, that's not relevant. I was looking for a flag that defaults to `True`. [Here's a better example](https://github.com/CUNY-CL/wikipron/blob/8c17aba81c87be530fbdb75aeb7d706754f2039c/src/wikipron/cli.py#L22).

> And, as usual, black is failing the test but passing locally. Sorry I can never remember what the culprit is :( `pip install -r requirements.txt` ought to set you...

> Ok will do. I think just `--tied_embedings/--no_...` right? Sure. I might create a section before the section on reserved symbols, call it `Tied embeddings`, and then say that by...

It's not in your PR description but this will close #142 and I think this will close #156 too.

It looks like we need to test if there are features, since I'm getting a breakage if there's not. I'll just modify to reflect...