search

salesforce / awd-lstm-lm

LSTM and QRNN Language Model Toolkit for PyTorch
BSD 3-Clause "New" or "Revised" License
1.96k stars 488 forks source link

Bug fix for the cuda option #6

Closed aykutfirat closed 6 years ago

aykutfirat commented 7 years ago

For argument "--cuda" "store_false" is inconsistent with the warning message. Updated the readme to have the cuda option as well.

salesforce-cla[bot] commented 7 years ago

Thanks for the contribution! Before we can merge this, we need @karimvision to sign the Salesforce Contributor License Agreement.

keskarnitish commented 7 years ago

Thanks for the PR; you are right about the inconsistency. Once the CLA is signed, @Smerity and I will merge it.

Smerity commented 7 years ago

Thanks so much for bringing this issue up :)

One thing to note, if we're going to fix the flag it might be best to go with --no-cuda instead? The "best" defaults are usually good ones so that users need to specify as little as possible. Using non-GPU is almost never going to be the way people will use this. This also has the added benefit of leaving all the README commands the same as before.

If this is a problem I can implement it at a later stage - main thing was you pointing out this documentation flaw, so thanks again ^_^