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

Make node replacement even easier #32

Open
liamhuber opened this issue Oct 16, 2023 · 0 comments
Open

Make node replacement even easier #32

liamhuber opened this issue Oct 16, 2023 · 0 comments

Comments

@liamhuber
Copy link
Member

liamhuber commented Oct 16, 2023

@JNmpi and I had a super short chat after the pyiron meeting tomorrow, and he was saying he'd still like to see syntax like:

# wf.min_phase1.calc = Macro.create.atomistics.CalcStatic  # Currently possible
wf.min_phase1.calc.replace_with.static()  # TODO, or similar

After introducing replacement and the Node.replace_with(self, other: Node) method in #26, this should actually be super easy to implement! A sketch looks like this: replace the replace_with method on Node with a property returning an instance of the following class:

class Replacer:
    def __init__(self, node: Node):
        self.node = node

    def __call__(self, other: Node):
        # This is just the current replacement code:
        if self.parent is not None:
            self.node.parent.replace(self, other)
        else:
            warnings.warn(f"Could not replace {self.node.label}, as it has no parent.")

Then a savvy developer can "power-up" their node with replacements they know to be compatible by overriding the replace_with property with an instance of a child of that class like:

class CalculatorReplacer(Replacer):
    @property
    def minimize(self):
        self(Workflow.create.atomistics.CalcMinimize)

    @property
    def static(self):
        self(Workflow.create.atomistics.CalcStatic)

    # Etc.

If the replacement is invalid this will just fail the way replacement normally fails -- e.g. if you have a minimize node with pressure connected to something, replacing with static should fail. I might need to double-check the tests and failure cases here to be sure this is working as intended, but the idea is perfectly straightforward.

As described above, this only works for nodes which are defined by actually making subclasses. We could probably stretch it to work using the decorators by extending the signature of Node to include replacer: Optional[type[Replacer]] and accept that as a decorator argument. Still requires the dev to be sophisticated enough to define a child-class of Replacer, but at least the decorators could still be used something like:

@Workflow.wrap_as.function_node(replacer=CalculatorReplacer)
def calc_static(job):
    # Blah blah

A bit of leg-work to get the examples and tests all set up, but I'm pretty happy with this direction.

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

No branches or pull requests

1 participant