Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attach functions to nodes #67

Open
JNmpi opened this issue Nov 5, 2023 · 2 comments
Open

Attach functions to nodes #67

JNmpi opened this issue Nov 5, 2023 · 2 comments
Labels
enhancement New feature or request

Comments

@JNmpi
Copy link

JNmpi commented Nov 5, 2023

Thanks again @liamhuber for the pull functionality. I tested it for the list_potentials example in my lammps node. It works really well. In the present implementation, the list_potentials node is part of the workflow and will thus be executed when running the workflow. A side effect of this behavior is that the output of this node appears in the output of the macro, which is not the desired behavior. What I would like to have is a functionality similar to your 'show' button in ironflow, where the available functions are only executed when explicitly called. A possible way to define such node functions would be to replace

structure = wf.create.lammps.Structure()
wf.list_pots = wf.create.lammps.ListPotentials(structure=structure)

with

structure = wf.add.lammps.Structure()
structure.attach.lammps.ListPotentials(structure=structure) 

or maybe even with

structure = wf.add.lammps.Structure()
structure.attach.lammps.ListPotentials() 

The property .attach allows us to attach functions to a specific node. With this, we have the flexibility to define class-like nodes that have next to input and output multiple node-specific functions. I have also the feeling that it would be more intuitive to replace create with add. This immediately symbolizes that the node is added to the workflow. It may be actually helpful to have also a create command that returns a standalone node, which is not attached to the workflow. Particularly when interactively working I missed such a feature a number of times. From the two alternatives given above, I would prefer the second (shorter) one, since it avoids having to specify the structure object twice (as generator and as function argument). The second version works also like the self-argument in a class and maybe thus more intuitive for Python users. However, its implementation may be more challenging.

Having this functionality would allow us to realize very easily a behavior as you have with show in ironflow. There, I find it a really powerful and highly useful concept and having something like

  structure.show.potentials()
  structure.show.source_code()
  structure.show.plot3d()

would be extremely helpful.

@JNmpi JNmpi added the enhancement New feature or request label Nov 5, 2023
@liamhuber
Copy link
Member

So I think this is actually two issues bundled together

create

I have also the feeling that it would be more intuitive to replace create with add.

Historically it was add, but then we exposed creation from the class level and added the ability to create executors (which is for now removed), and in both those cases add didn't make any sense.

This immediately symbolizes that the node is added to the workflow. It may be actually helpful to have also a create command that returns a standalone node, which is not attached to the workflow. Particularly when interactively working I missed such a feature a number of times.

This is already available! Workflow.create.... creates a clean node, while wf.create.... automatically parents it. So you always have access to a clean creator! Even inside a macro definition I suppose one could do something like macro.__class__.create to get this, although I don't see why you would. So there is no need to make a new tool for this.

It is, however, fair to ask if the workflow instances should have both create (not parented) and add (automatically parented). This would be easy enough to do, although the add menu would need to only have nodes and not other things like executors or a shortcut to the NotData class if those things become necessary.

I'm not closed to adding add alongside create, but I am also not convinced we have a burning need for the extra code.

Attaching functions

I like the direction, but I want to step back from your pseudocode for a second and think about the actual requirements

What I would like to have is a functionality similar to your 'show' button in ironflow, where the available functions are only executed when explicitly called.

With this, we have the flexibility to define class-like nodes that have next to input and output multiple node-specific functions.

It sounds here like what we really want is just extra methods on some node classes. I'm fully on board with that, but there's nothing here that requires the fanciness that comes with attach and show.

Did you perhaps also intend to have a requirement that these methods also be available as their own nodes sometimes? Without such a requirement, having extra methods is really easy when creating child classes:

Ex 1

class WithAttachments(SingleValue):
    def __init__(
        self,
        *args,
        label: Optional[str] = None,
        parent: Optional[Composite] = None,
        output_labels: Optional[str | list[str] | tuple[str]] = None,
        **kwargs,
    ):
        super().__init__(
            None,  # Override __init__ since we hard-code the node_function
            *args, 
            label=label,
            parent=parent,
            output_labels=output_labels,
            **kwargs
        )
        
    @staticmethod
    def node_function(n):
        import numpy as np
        array = np.arange(n)
        return array
    
    def plot(self):
        import matplotlib.pyplot as plt
        return plt.scatter(self.outputs.array.value)

I don't have the code for it, but I'm sure we can also include such things in the decorator construction by having the decorator add bound methods when it creates the dynamic class. I am 99.9% sure we can get this to work easily and be equivalent to above:

Ex 2

from pyiron_workflow import Workflow

def plot(self):
    import matplotlib.pyplot as plt
    return plt.scatter(self.outputs.array.value)

@Workflow.wrap_as.single_value_node("array", bound_methods=[plot])
def with_attachments(n):
    import numpy as np
    return np.arange(n)

If we wanted we can have some syntactic sugar like hiding these methods behind a show attribute, and we could even use that to check that the output data is ready before returning things.

I would prefer that we encourage node devs to do these sorts of things when defining a node class, but I guess it would also be possible to allow the dynamic binding of methods like some_node.show.bind(plot)

We could potentially get very fancy and have show try to do parsing so that it matches argument names in the bound method to node output values so that we could instead bind methods like

Ex 3

def plot(array):  # Name very carefully aligns with the name of an output channel!
    import matplotlib.pyplot as plt
    return plt.scatter(array)

Honestly, the only place where this feels like a strong advantage over Ex 2 is when the function we want to bind also comes from another node, e.g.

Ex 4

from pyiron_workflow import Workflow

@Workflow.wrap_as.single_value_node("fig")
def plot(array):
    import matplotlib.pyplot as plt
    return plt.scatter(sarray)

@Workflow.wrap_as.single_value_node("array", bound_methods=[plot.node_function])
def with_attachments(n):
    import numpy as np
    return np.arange(n)

But this would require the argument names to match the output labels (or to introduce yet another map), which I am not enthusiastic about.

Other than the suggestive pseudocode syntax of structure.attach.lammps.ListPotentials(), it doesn't really sound like this is what you had in mind anyhow, in which case Exs 1 and 2 would be sufficient.

@liamhuber
Copy link
Member

Actually, there might be a way for instance to have create and add separately that actually shrinks the code base, in which case I'm strongly in favour rather than apathetic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants