sd: support for JSON fields on the C++ library interface

[wbruna]

(this is on top of #1765 because I'll need new fields to test it, but only the last commit matters here)

This is a "demo" of using a JSON C++ parser to pack several image generation or model parameter fields into a single string at the Python - C++ interface level.

This is not intended to receive unfiltered content from the web server, just to make it easier to deal with the increasing complexity of the model and image parameters: optional fields, variant sets (clip_l versus clip_l+clip_g versus clip_l+t5xxl versus qwen2vl), nested parameters, etc. It also avoids the need of changing koboldcpp.py and expose.h in lockstep, and replicating all the field default values.

The current code isn't functional, just the parse_json_parameters function + auxiliary code showing what the parser could look like, and how new fields could be added, with debug and error output (set fields, parse errors, wrongly typed fields, and unknown fields). I've adopted a flat "field.subfield" approach, but nested objects would work too.

The test input and output:

{
    "vae_tiling_params.enabled": true,
    "vae_tiling_params.rel_size_x": 3.0,
    "vae_tiling_params.tile_size_y": "x",
    "vae_tiling_params.tile_size_x": 1.0,
    "ajgdhfsjdgfj": 12
}

json fields: vae_tiling_params.enabled=true
json fields: vae_tiling_params.tile_size_x=1
json fields: vae_tiling_params.rel_size_x=3.000000
json fields: vae_tiling_params.tile_size_y: wrong type (expected int)
json fields: ajgdhfsjdgfj: unmatched key

[LostRuins]

fyi before you put too much work into this, I think it might be a little overkill. my approach to image gen has generally favored simplicity with good defaults that are approachable for most people.

generally:

• if a setting is always beneficial to use, it should be hard coded.
• if it's critical for functionality but varies a lot and needs to be configured, it can be passed via API (e.g. step count)
• if it's non-critical but important to adjust for some use cases, it can be in the new --sdgendefaults
• if it's experimental, trivial or superfluous it should be considered out of scope and only adjusted by manually editing code

so like adjusting tiling size I would consider experimental - if we have something conclusively better we can adjust the defaults, otherwise it's probably a bit of unnecessary complexity to expose.

[wbruna]

Actually, this feature would mainly benefit us developers, not end users. A JSON dictionary would be very good at implementing optional or default parameters, or iterating through them.

Of course, you are the main project developer; if you don't see much value in this approach, it's a bit pointless to argue about it 🙂 But I'll try to explain my side anyway.

my approach to image gen has generally favored simplicity with good defaults that are approachable for most people.

I don't disagree with that, but code-wise, the current approach isn't that good at enabling experimentation: us developers finding out if a new parameter would add enough value to be changed or officially supported.

Take the tiling thing: I'm playing with that because the Wan VAE seems very tolerant to tiling, so a tuned tile size + overlapping factor could reduce the time spent on it maybe 30-40% for Qwen generations. Especially useful if the VAE needs to run on the CPU.

Problem is, I can't simply experiment changing the C++ side because I have an old machine, so build time quickly adds up. So, I turn to adding support to the fields... and that struct has five of them (not counting enabled). Five new entries in expose.h, with matching SDParams fields, and soon I'm getting a segfault because I got either the order or the types wrong on koboldcpp.py.

With an approach like this, I'd need five lines on the C++ side, letting the compiler figure out the correct types, and I'm free to either force values in koboldcpp.py, or whitelist them and use sdgendefaults. Single rebuild, no segfault, and the log tells me if I get a field name wrong. Better yet, I can use the prompt hack to experiment without even reloading the model, which is especially useful for a huge model like Qwen.

[LostRuins]

Are you doing a make clean with every build? Modifying the tiling value should only require rebuilding the sdcpp_default.o/sdcpp_vulkan.o file... which takes about 30 seconds to rebuild at most for me.

I do make LLAMA_VULKAN=1 -j 8 for my normal dev builds (you don't need to rebuild vulkan shaders/other files when changing sdcpp code)

I only do a make clean if something goes wrong e.g. new vulkan shaders added

[wbruna]

Are you doing a make clean with every build? Modifying the tiling value should only require rebuilding the sdcpp_default.o/sdcpp_vulkan.o file... which takes about 30 seconds to rebuild at most for me.

No; it's around 45s for me (assuming I get the change right on the first time). But these 45s add an extra pause (change the code and call make, wait, launch Koboldcpp and a gen, wait; versus (when no C++ change is needed) change the code, launch Koboldcpp and a gen, wait). And unless I hit an obvious all-users-will-need-this-value case, none of the needed C++ changes will make it to the final code. It's just a lot of overhead.

To add a real example: for the Nitro case, those 45s+pause added to maybe 10s of model loading time, for a ~2s test gen. Of course I tested it directly on sd.cpp (which at least has a command-line flag), until I came up with that prompt hack.

And to test the Qwen+tiling case, I'll use the prompt hack again, together with a generic JSON field, because Qwen also takes forever to load.

[LostRuins]

yup hmm i guess that's probably the best option for now, for self testing just hack it into the prompt and test locally

[wbruna]

Closing the PR, but I'll probably keep this branch around for testing.