candle icon indicating copy to clipboard operation
candle copied to clipboard

phi4 example does not work (cli args are swapped, model is not supported)

Open mcharytoniuk opened this issue 7 months ago • 2 comments

CLI arguments are swapped, so when I try to run the example with cargo run --example phi --release -- --prompt "hello" --model "4-mini", Phi-2-old loads instead:

https://github.com/huggingface/candle/blob/92106c8762dd7fdbd38aaf8e6555ef26dd59d5be/candle-examples/examples/phi/main.rs#L150-L153

I tried to swap them back, but then Phi-4 config format is incompatible with Phi3 Config:

https://huggingface.co/microsoft/Phi-4-mini-instruct/blob/618e63e60172e9efa181cbfeee9d7012e5b2ad66/config.json#L34-L38

  "rope_scaling": {
    "long_factor": [
      1,
      1.118320672,
      1.250641126,
...

vs

https://github.com/huggingface/candle/blob/92106c8762dd7fdbd38aaf8e6555ef26dd59d5be/candle-transformers/src/models/phi3.rs#L41

The actual type in the Phi-4 config is map, while Phi-3 config expects Option<String>. I tried removing rope_scaling field from Phi-4 JSON config as a workaround, as it is unused in Phi-3 runner anyway, but then I am getting an error Error: cannot find tensor lm_head.weight.

So seems like Phi-4 support is currently broken. I think it should be removed from canle-examples for now (https://github.com/huggingface/candle/pull/2790 should be reverted imo).

mcharytoniuk avatar May 19 '25 07:05 mcharytoniuk

@janimo do you think you could look at this ?

LaurentMazare avatar May 19 '25 07:05 LaurentMazare

@mcharytoniuk @LaurentMazare sorry about that, my bad. Looks like I did not test the model I thought I was testing... I hope I will have time to take a look these days. The patch can be reverted if you think so.

janimo avatar May 19 '25 18:05 janimo

No worries, I've added some proper support for phi-4 in #2960.

LaurentMazare avatar May 21 '25 08:05 LaurentMazare