facebookresearch / InferSent

InferSent sentence embeddings
Other
2.28k stars 471 forks source link

Hard-coded embedding dimension 300 #70

Closed Tiriar closed 6 years ago

Tiriar commented 6 years ago

Hello,

first of all, thank you for your work, I have used InferSent in my project and it boosted the performance quite a lot without much work :+1: .

I am currently trying to train my own InferSent model with different word embeddings, and I noticed a minor issue with the code. Even though I can change 'params.word_emb_dim' in 'train_nli.py', the parameter is not getting propagated to all functions it uses, where the dimension is hard-coded to 300. In order to use different embeddings, I had to manually change the number on several places throughout the project, making it quite cumbersome.

aconneau commented 6 years ago

Hi,

thanks for your message! hmm yeah that's indeed unfortunate. Could you modify the few lines of code that make this issue and push a PR? That would be great.

Thanks!

Tiriar commented 6 years ago

Sure, will do ;)

Tiriar commented 6 years ago

@aconneau Hello, just one addition, I finally got to training the model properly. When the training is finished and the training process gets to

# Run best model on test set.
nli_net.load_state_dict(os.path.join(params.outputdir, params.outputmodelname))

it throws an exception

Traceback (most recent call last):
  File "train_nli.py", line 292, in <module>
    nli_net.load_state_dict(os.path.join(params.outputdir, params.outputmodelname))
  File ".../anaconda3/lib/python3.6/site-packages/torch/nn/modules/module.py", line 508, in load_state_dict
    for name, param in state_dict.items():
AttributeError: 'str' object has no attribute 'items'

After a bit of digging I found that load_state_dict does not take a path to the model, but a loaded model. So I believe there should be an additional torch.load(os.path.join(...)) (I have tested it on a small network and it works this way).

I can add it to my pull request if you want (EDIT: I have added it now).

aconneau commented 6 years ago

Merged #73! Thanks!