Adriane Boyd

Results 347 comments of Adriane Boyd

My feeling is that the overhead probably isn't that high, we should profile it a bit for sample projects. Doing all the `requirements.txt` parsing is somewhat challenging, but the machinery...

I would only check for the packages in `requirements.txt` and ignore the full dependency check. It doesn't look like the dependencies in the conda packages are incorrect (it should be...

Why isn't `column` implemented in the model in the same way as in `HashEmbed`?

Both `HashEmbed` and the PR description look more like this: https://github.com/explosion/thinc/blob/40c129f6ee51d4060f1af6b29f903ce51fd89736/thinc/layers/hashembed.py#L48-L53 It's not that I think the output here is incorrect, but I'm trying to understand why it's implemented differently.

Ah, I see what you mean. I was mainly thinking that implementing this the same way would avoid a lot of the casting and numpy/cupy problems. In general I'm not...

I think that sounds reasonable.

This version looks good! It does change the parameters and types enough that I think it should be `remap_ids.v2`, though. I think we can just have both in parallel here?

I agree that the existing version is wonky, but the main issue is the parameters, since this removes `dtype` and adds `column`. If you hadn't removed the existing test it...

I don't think you need to spend much effort trying to clean up `.v1`. It can be buggy/broken and we wouldn't touch it again. For now I would propose keeping...

Also please update the corresponding docs: https://thinc.ai/docs/api-layers#remap_ids