[bug]: Nodes unable to connect ModelIdentifierField without a ui_type set to an input ModelIdentifierField with a ui_type set

[skunkworxdark]

Is there an existing issue for this problem?

• I have searched the existing issues

Operating system

Windows

GPU vendor

Nvidia (CUDA)

GPU model

No response

GPU VRAM

No response

Version number

4.2.1

Browser

chrome

Python dependencies

No response

What happened

While using my custom nodes in my XYGrid nodes pack. You are prevented from connecting an output ModelIdentifierField (without a ui_type set) to an input ModelIdentifierField field that has a ui_type set.

This was reported as a bug on my nodes pack today. skunkworxdark/XYGrid_nodes#6

What you expected to happen

It should allow you to connect them as the base type is the same.

How to reproduce the problem

No response

Additional context

No response

Discord username

skunkworxdark

[psychedelicious]

ui_type strictly overrides any inferred type based on pydantic annotations.

The name ui_type is perhaps misleading. It sounds like it means this:

The client will display UI element for this type, but still use the python type annotations for connection validation.

But what it actually means is this:

The client will use this type instead of the python type annotations, both for UI elements and connection validation. The python type annotation is ignored by the client.

In this case, the main_model_loader_input node's model field uses ui_type to explicitly set the type to UIType.MainModel:
https://github.com/skunkworxdark/XYGrid_nodes/blob/main/images_to_grids.py#L264

The string_to_main_model node outputs a ModelIdentifierField. It doesn't override the type: https://github.com/skunkworxdark/XYGrid_nodes/blob/main/images_to_grids.py#L290

So it's worked exactly as expected right now. ModelIdentifierField is not compatible with MainModelField.

To resolve this, we could add rudimentary subtyping for model fields. We already do this for float and int, where int is considered a subtype of float for the purposes of connection validation.

For example, UIType.MainModelField would be a subtype of ModelIdentifierField, and so anything that accepts ModelIdentifierField also accepts UIType.MainModelField - or any of the UIType.SomethingModelField types.

[psychedelicious]

I mixed up which is an input and which is an output in your nodes.

If the input is UIType.MainModelField, it cannot accept ModelIdentifierField, even with subtyping. UIType.MainModelField is narrower than ModelIdentifierField - we cannot allow the narrower-typed input to accept a wider-typed value. So my idea about subtypes won't fix this particular case.

Ok, another idea - we can track the python-source types alongside any type override specified with ui_type, and use that as a fallback for validation.

[skunkworxdark]

Thanks for the explanation it does clarify my understanding of the ui_type. Tracking the Python type alongside the overridden type makes the most sense to me. But is it useful to other nodes or cause issues?

I am far away from the same oversight of the project as you are and worry that it might introduce other problems that are not immediately obvious. The most obvious thing is the reduction in strict type-checking in the UI but in this case, the worst is the model can't be found and loaded (I think!!!).

There are multiple other options available that I could look into from the node end of things that would require no change to the core:

• With the string_to_main_model node create multiple versions, one for each main model type. Or keep it as a single node with each model type as individual outputs. These both feel a bit messy.
• Have a new node(s) that converts between ModelIdentifierField and the model ui types. Again this feels wasteful.
• With the main_model_loader_input and SDXL version remove the ui_type and let it be just a ModelIdentifierField type. I would lose the option to select a model via the UI and be restricted to connection-only inputs.
• I could combine the string_to_main_model and the model_loader_input nodes and just have the model_loader have a string input. Currently, I subclass the ModelLoaderInvocation, tracking any changes made in the core. I am not 100% sure I could still do that this way around, it would need some experimentation.