IDAES/idaes-pse

Add Pylint Check for Lambda Functions

Closed this issue · 1 comments

In the course of debugging an issue with collocation, @jsiirola pointed out that lambda functions do not behave well with copy.deepcopy, which can prevent Pyomo from cloning models correctly.

>>> a = [1,2,3]
>>> def foo(x):
...    print(id(x), x)
...
>>> xx = lambda: foo(a)
>>> yy = copy.deepcopy(xx)
>>> xx()
140447310398160 [1, 2, 3, 5]
>>> yy()
140447310398160 [1, 2, 3, 5]
>>> a.append(6)
>>> xx()
140447310398160 [1, 2, 3, 5, 6]
>>> yy()
140447310398160 [1, 2, 3, 5, 6]

It is unclear to the user that a is not going to be deep copied as well (which probably relates to closure rules). The preferred way of accomplishing something like this is using functools.partial.:

>>> import functools
>>> x = functools.partial(foo, a)
>>> import copy
>>> y = copy.deepcopy(x)
>>> x()
140447310398160 [1, 2, 3]
>>> y()
140447178509520 [1, 2, 3]
>>> a.append(5)
>>> x()
140447310398160 [1, 2, 3, 5]
>>> y()
140447178509520 [1, 2, 3]

Another example occurs when trying to use lambda functions to reduce the function's signature as part of a loop:

def rule_flow_mol_comp(blk, t, j, props, port):
    return (
        props[t].flow_mol_comp[j]
        == blk.number_cells * port.flow_mol[t] * port.mole_frac_comp[t, j]
    )
for side in ["fuel", "oxygen"]:
    side_comps = getattr(self.solid_oxide_cell, f"{side}_component_list")
    for direction in ["in", "out"]:
        props = getattr(self, f"{side}_properties_{direction}")
        port_name = f"{side}_{direction}let"  # e.g., fuel_inlet, oxygen_outlet
        port = getattr(self.solid_oxide_cell, port_name)
        setattr(
            self,
            f"{port_name}_flow_mol_comp_eqn",
            pyo.Constraint(
                tset,
                side_comps,
                rule=lambda blk, t, j: rule_flow_mol_comp(
                    blk, t, j, props, port
                ),
        )

This code correctly generates the ports with the proper equations the first time through, but when the rule is called again when generating additional timepoints for PyomoDAE it uses the last values of props and port looped over, regardless of what was used originally. We have determined that the correct way to implement something like this is to use functools.partial:

from functools import partial

def rule_flow_mol_comp(blk, t, j, props, port):
    return (
        props[t].flow_mol_comp[j]
        == blk.number_cells * port.flow_mol[t] * port.mole_frac_comp[t, j]
    )

for side in ["fuel", "oxygen"]:
    side_comps = getattr(self.solid_oxide_cell, f"{side}_component_list")
    for direction in ["in", "out"]:
        props = getattr(self, f"{side}_properties_{direction}")
        port_name = f"{side}_{direction}let"  # e.g., fuel_inlet, oxygen_outlet
        port = getattr(self.solid_oxide_cell, port_name)
        setattr(
            self,
            f"{port_name}_flow_mol_comp_eqn",
            pyo.Constraint(
                tset,
                side_comps,
                rule=partial(rule_flow_mol_comp, props=props, port=port),
        )

Given that, and given that Pylint already discourages lambda functions because they don't get named in the stack trace, it's probably worthwhile to add a test rejecting code with lambdas in it.

We probably need to re-enable to check for lambdas in the pylintrc file, and then go and check all the places they are used.