convert.py : Get rope scale from HuggingFace models

[pnb]

I was trying to convert EverythingLM V2 with 16k context to GGUF and noticed that it generated nonsense. GGUF metadata showed that the rope scale was not kept, and I see it was indeed not read from config.json. This PR fixes that. The rope scale shows up in GGUF metadata (as 0.25 in this case) and model output is now coherent.

Suggesting @slaren for review because I saw you recently made a very related change.

[slaren]

I am looking at the config.json of the model that you linked, and the rope scale looks like this:

  "rope_scaling": {
    "factor": 4.0,
    "type": "linear"
  },

This PR seems to ignore the type parameter. It would be good to add a check to make sure that it is linear.

[pnb]

Makes sense! I added that check now, and tested it to make sure it is ignored if the type = linear check fails (and that it still works if type is linear).

[cebtenzzre]

nit: dict.get returns None as a fallback by default. This would be more readable:

Suggested change

	f_rope_scale = config.get("rope_scaling", {}).get("factor", None) if config.get("rope_scaling", {}).get("type", "") == "linear" else None
	rope_scaling = config.get("rope_scaling", {})
	f_rope_scale = rope_scaling.get("factor") if rope_scaling.get("type") == "linear" else None

[slaren]

I would be happy with even a couple of plain if statements. But yeah, that one-liner a bit hard to read, anything that improves the readability would be good.

[pnb]

I was overfitting a bit to the other implementations trying to match the style. I am not a fan of tertiary operators or long lines, so I would also prefer an if/else implementation (tested and committed now).

[pnb]

One other thing I had thought of is whether it is worth raising an Exception if type is not "linear", rather than silently ignoring rope scaling. I didn't implement that because it doesn't seem like the rest of the code cares about nonlinear scaling, so it appears kind of out of scope -- but I'll add it if that seems like something that could come up.

Edit: On further thought, it could take a while to see where people converge on rope scaling methods. I get the sense linear might win out. So I think it is fine to merge now, since this fixes a lot of models on HF.

[TheBloke]

Hey guys, just tried convert.py with a model with no rope_scaling defined and got this error:

    if "rope_scaling" in config and config["rope_scaling"].get("type") == "linear":
AttributeError: 'NoneType' object has no attribute 'get'

I'd suggest to use this code instead:

rope_scaling = config.get("rope_scaling", None)
if isinstance(rope_scaling, dict) and rope_scaling.get("type", None) == "linear":
   f_rope_scale = rope_scaling.get("factor", None)
else:
   f_rope_scale = None

[pnb]

Strange! What model was this? I do suspect there might be some variance in how rope scaling appears in config.json that could be better handled. If rope_scaling is not a dict, for example, which I gather might be the case from your example code.

[TheBloke]

It was Samantha-1.11-70B, but what happened here is the default case. Most models currently out there don't have any rope scaling defined. The default in config.json is:

    "rope_scaling": null,

and that wasn't being handled with the code as it was.

[pnb]

Ah that makes sense. Will submit the fix shortly, as soon as I try it on a couple models.