luyug / COIL

NAACL2021 - COIL Contextualized Lexical Retriever
Apache License 2.0
142 stars 27 forks source link

Small typo and bug? #2

Closed MXueguang closed 3 years ago

MXueguang commented 3 years ago

Hi @luyug, Great work!!!!

I am trying to replicate COIL, there is some typo I noticed.

  1. in README.md Encoding section

    --token_dim 768 \  
    --cls_dim 32 \ 

    should be 32, 768 instead?

  2. https://github.com/luyug/COIL/blob/813a076ad7526536dad5d4fc71eee5f7f8113700/trainer.py#L42 num_training_steps should be passed into the function rather than warmup_steps?

luyug commented 3 years ago

Thanks for catching these! Will push a fix asap.

I am super surprised that the second typo happened during the refactor. It actually got me worried and I went back to check all of my other repos that have the same function call. Fortunately they all seem correct.. I guess it is just a bad idea to multi-task on too many things at the same time..

MXueguang commented 3 years ago

closing the issue, since (2) get fixed