Change switch creation API

[sapetnioc]

I am trying to fix a file completion issue that is related to the use of ̀̀Switch nodes. I think we can solve this problem and simplify the switch definition API at the same time. The problems comes from the fact that switch nodes are created before they are connected to the plugs of other nodes. Therefore, their output plugs are created without knowing the type of the corresponding input plugs. If this type is not given by user, the type Any is used. The actual type of the input plug (the one that is actually connected to the output plug for a given switch state) is lost and this makes the path name completion fail because it works only on parameters with a known File or Directory type.

I propose to change the API of switch creation to avoid separation between the creation of plugs and the connection of nodes on input. This way internal switch API would always know at least one existing input plug when creating an output plug. Moreover, I think that the new API is much simpler because it hides this strange <input>_switch_<output> parameters. Let's take an example, the following switch creation with two inputs/outputs is used in Capsul tests:

        # Create processes
        self.add_process(
            "way1",
            "capsul.sphinxext.test.test_process_pipeline_doc.MyProcess")
        self.add_process(
            "way2",
            "capsul.sphinxext.test.test_process_pipeline_doc.MyProcess")
        self.add_process(
            "node",
            "capsul.sphinxext.test.test_process_pipeline_doc.MyProcess")

        # Link input
        self.export_parameter("way1", "input_image")
        self.export_parameter("way1", "input_float")
        self.add_link("input_image->way2.input_image")
        self.add_link("input_float->way2.input_float")

        # Create Switch
        self.add_switch("switch", ["one", "two"],
                        ["image", "float", ])

        # Link way1
        self.add_link("way1.output_image->switch.one_switch_image")
        self.add_link("way1.output_float->switch.one_switch_float")

        # Link way2
        self.add_link("way2.output_image->switch.two_switch_image")
        self.add_link("way2.output_float->switch.two_switch_float")

I propose to change the switch creation part (starting from # Create Switch) with the following code:

        # Create Switch
        switch = self.create_switch("way1")

        # Link way1
        switch.add_option("one",
                          image="way1.output_image",
                          float="way1.output_float")

        # Link way2
        switch.add_option("two",
                          image="way2.output_image",
                          float="way2.output_float")

I believe this API change can be implemented without changing current Switch node. The object returned by create_switch() would not be a real Node but a kind of switch factory. Not only, this change would allow me to implement a first solution to my plug typing problem (for instance by using the type of the input plug given in the first add_option() instead of Any). But I think the code is also much more easy to read and understand.

[denisri]

I completely agree that the Any type in the current implementation of switches is a problem. Your solution is OK for from a programmer point of view. But it would cause a problem in the pipeline creation GUI: this new "dynamic" switch would add plugs when connecting nodes, but the GUI only allows to connect existing plugs, so it would require a relatively important modification in the GUI manipulation.

[sapetnioc]

You are right that the proposed API does not make it easy to implement a graphical interface. I propose to keep both the programmer API and the internal representation with an explicit link between them. The internal representation would be very close to the current one (with a few changes like imposing plug typing at Switch creation). This API would be used by graphical interface. This internal representation would be transformed in programmer API when exported in Python format.

[sapetnioc]

I had to change the proposed API because it is very difficult to modify the type of the switch parameter once it is exported since each exportation may create a copy of the field. Since the type contains the possible values for the switch, it is necessary to know all these values when creating the switch. Therefore, the new API uses only one method. The above example becomes:

        self.create_switch("way1",
            options={
                "one": {
                    "image": "way1.output_image",
                    "float": "way1.output_float",
                },
                "two": {
                    "image": "way2.output_image",
                    "float": "way2.output_float",
                },
            }
        )

The new API is implemented and made it possible to fix the completion problem. Therefore, I changed all examples and tests to use the new API. add_switch should not be used anymore because it is not compatible with file names completion.

[denisri]

So now, the switch is created already connected, right ? It's probably not a problem for programmers, but how will the GUI be used to create a switch ? The way we create it is totally different from what it was before...

I also see that you have modified FakeMorphologist to match the new API, it's good for the example, but not for the meaning of FakeMorphologist: this pipeline is an automatic conversion of the real morphologist pipeline, and was not meant to be modified manually (see https://github.com/populse/capsul/blob/3.0/capsul/pipeline/test/fake_morphologist/README.rst) because its aim is to test that the conversion works, which does not at the moment ;) However your modifs will certainly help me to adapt the converters.

[denisri]

The new switches definition makes it difficult to code pipeline saving or conversion. I don't know how does the save behaves right now (I haven't tried yet) but I have problems with pipeline conversions from Axon. In v2 the capsul pipeline was written in a straightforward order:

• declare all nodes (add_process, add_switch...)
• export nodes parameters
• link all nodes plugs

Within these 3 steps, the nodes or plugs order did not matter at all.

In v3 with switches it's not so straightforward: to be able to declare a switch node,

• other needed nodes must be declared first
• if several switches are connected together, they must be declared in the correct order
• exports must be done for these nodes
• then a new switch can be added, needing already declared nodes (including dependent switches), and exported plugs
• then new switches plugs may be exported or connected

Thus we have to completely follow a topological order to declare things, and correctly alternate nodes declaration and plugs exportations. It's not impossible to implements, but significantly complicates the process.

Does pipeline saving implement all this ?

[denisri]

Does pipeline saving implement all this ?

Answer: it does not: pipeline saving is broken, at least when a switch is involved.

[denisri]

Well, I fail to write export code for the new switches. Not that it is impossible, but it requires to rewrite a large part of the exportation logics and needs some time (and some thinking) to write it. At this point, I feel it's easier to implement a more "dynamic" switch which would follow your first proposition, or could behave like in capsul v2 with "dynamic typing": the switch may be created unconnected, with no plugs, or with existing plugs, and the plugs would be created or their type modified when connecting nodes. What was blocking for you @sapetnioc, is changing the type of plugs dynamically. I think it can be implemented by removing the plug, re-creating it with a different type, and reconnecting already existing connections, if there are (which should normally not). In Capsul, connections do not really check that they link plugs with the same type, and things would thus not change for other nodes. The only case when we should need to "propagate" the new type through links is a switch node already exported to its parent pipeline. Not too difficult to implement, I think.

[sapetnioc]

Well, I first tried to do as you said, change the plug type but it is more complicated than it looks. Switch parameters can be exported several times (pipeline in pipeline) and the exported plug is the place where the actual type is needed for completion. This type is taken from the field, therefore it is the field type that must be changed. The field itself cannot be changed, dataclass instances are immutable. Removing and re-creating dynamic field looses all callbacks. And finally an exported become simply a pipeline parameter with a link. It is difficult to tell the links that correspond to an export, and therefore should propagate the field type change, from links that are simple links.

I still believe that it should be possible to have both API working by fixing Python exportation. It would require to write switch nodes after processes nodes, it should be enough to handle dependencies unless we connect a switch output to another switch input in a the same pipeline (in that case, switches must be ordered according to their links).

We will have to discuss about the best options. I also probably should learn how the generation of FakeMorphologist work to be able to start it before running tests.