google / gemma_pytorch

The official PyTorch implementation of Google's Gemma models
https://ai.google.dev/gemma
Apache License 2.0
5.24k stars 499 forks source link

--quant always returns True #5

Closed nakkapeddi closed 6 months ago

nakkapeddi commented 6 months ago

Regardless of what value is passed in, the --quant argument will always return True. Python will cast the string to always true, even if you have set it to false:

$ docker run -t --rm     --gpus all     -v ${CKPT_PATH}:/tmp/ckpt     ${DOCKER_URI}     python scripts/run.py     --device=cuda     --ckpt=/tmp/ckpt  --quant="false" --variant="${VARIANT}"  --prompt="${PROMPT}"                                                                                                                  
True

>>> bool("foo")
True
>>> bool("false")
True
>>> bool("0")
True

Solution is to have a portable strtobool utility, and I suggest having a contract of something like "0", "1", "true", "false". Bash is stringly typed and Python is duck typed, so it leads to this kind of issues.

pengchongjin commented 6 months ago

Hmm, can you try --quant=false without the double quotes? I think the double quote might be problematic.

pengchongjin commented 6 months ago

Looks like simply setting --quant=false didn't work.

pengchongjin commented 6 months ago

I pushed a fix here. The solution is to make --quant an bool flag by setting the action='store_true'. Please let us know if that work for you. Thanks!

https://github.com/google/gemma_pytorch/commit/5f2c166bd7bdd24daa868a827cfbec3bb5405ef3

ghost commented 6 months ago

Replace parser.add_argument("--quant", type=bool, default=False) in run.py with parser.add_argument("--quant", action='store_true'). As a result, if you include --quant on the command line, args.quant will be set to True. If you do not include --quant, then args.quant will be set to False. Here is an example of using the quantified gemma model:

python scripts/run.py --device=cuda --ckpt=/tmp/ckpt --variant="2b" --quant --prompt="Hi, gemma."
michaelmoynihan commented 6 months ago

Thanks for the fix, Pengchong! Looks like this can be closed.