Mutable default arguments

[eslickj]

I found a problem with a table utility function and I'm afraid it could be an issue elsewhere in IDAES. I know in quite a few places we may use an empty list or dict as a default. The problem with a mutable default argument is that if you change the augment in the function, you also change the default value. Here's and example:

>>> import copy
>>> def f(k, x={}):
...   x[k] = "Hi"
...   return copy.copy(x)
...
>>> print(f(1))
{1: 'Hi'}
>>> print(f(2))
{1: 'Hi', 2: 'Hi'}
>>> print(f(3))
{1: 'Hi', 2: 'Hi', 3: 'Hi'}

I'm not sure if we are making a similar mistake anywhere else, but we should probably check. I know we had a lot of mutable objects as default values, but I don't know if we alter them in the function. I wonder if this is something pylint can do or does.

[ksbeattie]

@eslickj, this can be caught by pylint. Given the way we are running pylint in our tests right now, it won't but here's how to search just for that specific warning:

pylint --disable=all --enable=dangerous-default-value idaes

I just ran it and found...

A lot of warnings

$ pylint --disable=all --enable=dangerous-default-value idaes
************* Module idaes.core.unit_model
idaes/core/unit_model.py:608:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.core.control_volume1d
idaes/core/control_volume1d.py:153:4: W0102: Dangerous default value [] as argument (dangerous-default-value)
idaes/core/control_volume1d.py:1637:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.core.control_volume0d
idaes/core/control_volume0d.py:1320:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.core.util.scaling
idaes/core/util/scaling.py:529:4: W0102: Dangerous default value [] as argument (dangerous-default-value)
************* Module idaes.core.util.misc
idaes/core/util/misc.py:35:0: W0102: Dangerous default value {} as argument (dangerous-default-value)
idaes/core/util/misc.py:118:0: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.core.util.initialization
idaes/core/util/initialization.py:36:0: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.core.util.tables
idaes/core/util/tables.py:27:0: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.core.util.phase_equilibria
idaes/core/util/phase_equilibria.py:39:0: W0102: Dangerous default value {} as argument (dangerous-default-value)
idaes/core/util/phase_equilibria.py:76:0: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.core.util.model_serializer
idaes/core/util/model_serializer.py:392:0: W0102: Dangerous default value {} as argument (dangerous-default-value)
idaes/core/util/model_serializer.py:392:0: W0102: Dangerous default value [] as argument (dangerous-default-value)
idaes/core/util/model_serializer.py:442:0: W0102: Dangerous default value {} as argument (dangerous-default-value)
idaes/core/util/model_serializer.py:442:0: W0102: Dangerous default value [] as argument (dangerous-default-value)
idaes/core/util/model_serializer.py:536:0: W0102: Dangerous default value {} as argument (dangerous-default-value)
idaes/core/util/model_serializer.py:618:0: W0102: Dangerous default value {} as argument (dangerous-default-value)
idaes/core/util/model_serializer.py:618:0: W0102: Dangerous default value {} as argument (dangerous-default-value)
idaes/core/util/model_serializer.py:661:0: W0102: Dangerous default value {} as argument (dangerous-default-value)
idaes/core/util/model_serializer.py:661:0: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.generic_models.properties.core.generic.generic_property
idaes/generic_models/properties/core/generic/generic_property.py:953:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
idaes/generic_models/properties/core/generic/generic_property.py:953:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.generic_models.properties.cubic_eos.cubic_prop_pack
idaes/generic_models/properties/cubic_eos/cubic_prop_pack.py:212:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.generic_models.properties.activity_coeff_models.activity_coeff_prop_pack
idaes/generic_models/properties/activity_coeff_models/activity_coeff_prop_pack.py:228:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
idaes/generic_models/properties/activity_coeff_models/activity_coeff_prop_pack.py:228:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.generic_models.properties.examples.saponification_thermo
idaes/generic_models/properties/examples/saponification_thermo.py:122:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
idaes/generic_models/properties/examples/saponification_thermo.py:122:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.generic_models.unit_models.separator
idaes/generic_models/unit_models/separator.py:1326:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.generic_models.unit_models.statejunction
idaes/generic_models/unit_models/statejunction.py:115:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
idaes/generic_models/unit_models/statejunction.py:115:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.generic_models.unit_models.heat_exchanger_1D
idaes/generic_models/unit_models/heat_exchanger_1D.py:606:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.generic_models.unit_models.pressure_changer
idaes/generic_models/unit_models/pressure_changer.py:643:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.generic_models.unit_models.feed
idaes/generic_models/unit_models/feed.py:110:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.generic_models.unit_models.heat_exchanger
idaes/generic_models/unit_models/heat_exchanger.py:434:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.generic_models.unit_models.product
idaes/generic_models/unit_models/product.py:131:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
idaes/generic_models/unit_models/product.py:131:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.generic_models.unit_models.translator
idaes/generic_models/unit_models/translator.py:195:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
idaes/generic_models/unit_models/translator.py:195:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
idaes/generic_models/unit_models/translator.py:195:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.generic_models.unit_models.mixer
idaes/generic_models/unit_models/mixer.py:818:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.generic_models.unit_models.valve
idaes/generic_models/unit_models/valve.py:169:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.generic_models.unit_models.distillation.tray
idaes/generic_models/unit_models/distillation/tray.py:645:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.generic_models.unit_models.distillation.tray_column
idaes/generic_models/unit_models/distillation/tray_column.py:366:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.generic_models.unit_models.distillation.condenser
idaes/generic_models/unit_models/distillation/condenser.py:696:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.generic_models.unit_models.distillation.reboiler
idaes/generic_models/unit_models/distillation/reboiler.py:563:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.generic_models.control.tests.test_steam_tank
idaes/generic_models/control/tests/test_steam_tank.py:85:0: W0102: Dangerous default value [] as argument (dangerous-default-value)
************* Module idaes.gas_solid_contactors.properties.methane_iron_OC_reduction.solid_phase_thermo
idaes/gas_solid_contactors/properties/methane_iron_OC_reduction/solid_phase_thermo.py:216:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.gas_solid_contactors.properties.methane_iron_OC_reduction.gas_phase_thermo
idaes/gas_solid_contactors/properties/methane_iron_OC_reduction/gas_phase_thermo.py:239:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.gas_solid_contactors.properties.methane_iron_OC_reduction.hetero_reactions
idaes/gas_solid_contactors/properties/methane_iron_OC_reduction/hetero_reactions.py:223:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.gas_solid_contactors.unit_models.moving_bed
idaes/gas_solid_contactors/unit_models/moving_bed.py:803:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
idaes/gas_solid_contactors/unit_models/moving_bed.py:803:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
idaes/gas_solid_contactors/unit_models/moving_bed.py:803:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.gas_solid_contactors.unit_models.bubbling_fluidized_bed
idaes/gas_solid_contactors/unit_models/bubbling_fluidized_bed.py:1520:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
idaes/gas_solid_contactors/unit_models/bubbling_fluidized_bed.py:1520:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
idaes/gas_solid_contactors/unit_models/bubbling_fluidized_bed.py:1520:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.convergence.power_generation.heater
idaes/convergence/power_generation/heater.py:39:0: W0102: Dangerous default value [] as argument (dangerous-default-value)
************* Module idaes.surrogate.helmet.Plotting
idaes/surrogate/helmet/Plotting.py:335:0: W0102: Dangerous default value [] as argument (dangerous-default-value)
idaes/surrogate/helmet/Plotting.py:335:0: W0102: Dangerous default value [] as argument (dangerous-default-value)
idaes/surrogate/helmet/Plotting.py:522:0: W0102: Dangerous default value [] as argument (dangerous-default-value)
idaes/surrogate/helmet/Plotting.py:522:0: W0102: Dangerous default value [] as argument (dangerous-default-value)
idaes/surrogate/helmet/Plotting.py:654:0: W0102: Dangerous default value [] as argument (dangerous-default-value)
idaes/surrogate/helmet/Plotting.py:654:0: W0102: Dangerous default value [] as argument (dangerous-default-value)
idaes/surrogate/helmet/Plotting.py:832:0: W0102: Dangerous default value [] as argument (dangerous-default-value)
idaes/surrogate/helmet/Plotting.py:832:0: W0102: Dangerous default value [] as argument (dangerous-default-value)
idaes/surrogate/helmet/Plotting.py:982:0: W0102: Dangerous default value [] as argument (dangerous-default-value)
idaes/surrogate/helmet/Plotting.py:982:0: W0102: Dangerous default value [] as argument (dangerous-default-value)
************* Module idaes.surrogate.pysmo.kriging
idaes/surrogate/pysmo/kriging.py:34:4: W0102: Dangerous default value [] as argument (dangerous-default-value)
idaes/surrogate/pysmo/kriging.py:34:4: W0102: Dangerous default value [] as argument (dangerous-default-value)
************* Module idaes.dmf.model_data
idaes/dmf/model_data.py:201:0: W0102: Dangerous default value {} as argument (dangerous-default-value)
idaes/dmf/model_data.py:201:0: W0102: Dangerous default value [] as argument (dangerous-default-value)
idaes/dmf/model_data.py:201:0: W0102: Dangerous default value {} as argument (dangerous-default-value)
idaes/dmf/model_data.py:478:0: W0102: Dangerous default value [] as argument (dangerous-default-value)
idaes/dmf/model_data.py:570:0: W0102: Dangerous default value [] as argument (dangerous-default-value)
************* Module idaes.power_generation.properties.flue_gas_ideal
idaes/power_generation/properties/flue_gas_ideal.py:324:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
idaes/power_generation/properties/flue_gas_ideal.py:324:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.power_generation.unit_models.boiler_heat_exchanger
idaes/power_generation/unit_models/boiler_heat_exchanger.py:1131:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
idaes/power_generation/unit_models/boiler_heat_exchanger.py:1131:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
idaes/power_generation/unit_models/boiler_heat_exchanger.py:1131:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.power_generation.unit_models.drum
idaes/power_generation/unit_models/drum.py:420:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
idaes/power_generation/unit_models/drum.py:420:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
idaes/power_generation/unit_models/drum.py:420:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.power_generation.unit_models.drum1D
idaes/power_generation/unit_models/drum1D.py:1022:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
idaes/power_generation/unit_models/drum1D.py:1022:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
idaes/power_generation/unit_models/drum1D.py:1022:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.power_generation.unit_models.boiler_heat_exchanger_2D
idaes/power_generation/unit_models/boiler_heat_exchanger_2D.py:2325:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.power_generation.unit_models.heat_exchanger_3streams
idaes/power_generation/unit_models/heat_exchanger_3streams.py:469:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.power_generation.unit_models.boiler_fireside
idaes/power_generation/unit_models/boiler_fireside.py:920:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.power_generation.unit_models.waterpipe
idaes/power_generation/unit_models/waterpipe.py:347:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.power_generation.unit_models.steamheater
idaes/power_generation/unit_models/steamheater.py:617:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.power_generation.unit_models.waterwall_section
idaes/power_generation/unit_models/waterwall_section.py:961:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.power_generation.unit_models.watertank
idaes/power_generation/unit_models/watertank.py:311:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
idaes/power_generation/unit_models/watertank.py:311:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.power_generation.unit_models.downcomer
idaes/power_generation/unit_models/downcomer.py:316:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.power_generation.unit_models.helm.turbine_outlet
idaes/power_generation/unit_models/helm/turbine_outlet.py:103:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
idaes/power_generation/unit_models/helm/turbine_outlet.py:103:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.power_generation.unit_models.helm.pump
idaes/power_generation/unit_models/helm/pump.py:137:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.power_generation.unit_models.helm.splitter
idaes/power_generation/unit_models/helm/splitter.py:263:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.power_generation.unit_models.helm.turbine_stage
idaes/power_generation/unit_models/helm/turbine_stage.py:70:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.power_generation.unit_models.helm.turbine_inlet
idaes/power_generation/unit_models/helm/turbine_inlet.py:125:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
idaes/power_generation/unit_models/helm/turbine_inlet.py:125:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.power_generation.unit_models.helm.turbine_multistage
idaes/power_generation/unit_models/helm/turbine_multistage.py:629:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.power_generation.unit_models.helm.compressor
idaes/power_generation/unit_models/helm/compressor.py:149:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.power_generation.unit_models.helm.turbine
idaes/power_generation/unit_models/helm/turbine.py:163:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.power_generation.unit_models.helm.mixer
idaes/power_generation/unit_models/helm/mixer.py:391:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.power_generation.unit_models.helm.phase_separator
idaes/power_generation/unit_models/helm/phase_separator.py:133:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
idaes/power_generation/unit_models/helm/phase_separator.py:133:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.power_generation.unit_models.helm.condenser_ntu
idaes/power_generation/unit_models/helm/condenser_ntu.py:306:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.power_generation.unit_models.helm.valve_steam
idaes/power_generation/unit_models/helm/valve_steam.py:228:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.power_generation.carbon_capture.mea_solvent_system.properties.vapor_prop
idaes/power_generation/carbon_capture/mea_solvent_system/properties/vapor_prop.py:281:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.power_generation.carbon_capture.mea_solvent_system.properties.liquid_prop
idaes/power_generation/carbon_capture/mea_solvent_system/properties/liquid_prop.py:466:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.power_generation.carbon_capture.mea_solvent_system.unit_models.phe
idaes/power_generation/carbon_capture/mea_solvent_system/unit_models/phe.py:716:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
************* Module idaes.power_generation.carbon_capture.mea_solvent_system.unit_models.column
idaes/power_generation/carbon_capture/mea_solvent_system/unit_models/column.py:1041:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
idaes/power_generation/carbon_capture/mea_solvent_system/unit_models/column.py:1041:4: W0102: Dangerous default value {} as argument (dangerous-default-value)
idaes/power_generation/carbon_capture/mea_solvent_system/unit_models/column.py:1041:4: W0102: Dangerous default value [] as argument (dangerous-default-value)
idaes/power_generation/carbon_capture/mea_solvent_system/unit_models/column.py:1041:4: W0102: Dangerous default value [] as argument (dangerous-default-value)
idaes/power_generation/carbon_capture/mea_solvent_system/unit_models/column.py:1041:4: W0102: Dangerous default value {} as argument (dangerous-default-value)

[ksbeattie]

For ref: https://docs.python-guide.org/writing/gotchas/#mutable-default-arguments

[jsiirola]

The "standard" python approach of

def fcn(arg=None):
    if arg is None:
        arg = {}

works well, except when None is a viable user-specified option. In that case, Pyomo settled on the convention of using a "throw away type":

class NoArgument(object):
   pass
def fcn(arg=NoArgument):
    if arg is NoArgument:
        arg = {}

The only downside of this is that Sphinx will expand the fully-qualified class name when rendering the function documentation, e.g., to something like:

fcn(arg=<class 'idaes.generic_models.unit_models.distillation.tray.NoArgument'>)

To work around this, Pyomo added a FlagType metaclass that generates a more reasonable __str__ / __repr__ of the type. There is also a "standard" NOTSET type that we are attempting to standardize on:

from pyomo.common.modeling import NOTSET
def fcn(arg=NOTSET):
    if arg is NOTSET:
        arg = {}

which generates Sphinx documentation similar to

fcn(arg=NOTSET)

[ksbeattie]

For this we still need a check by pylint.

[andrewlee94]

@lbianchi-lbl @ksbeattie We should enable pylint checking for this ASAP to make sure it doesn't start creeping back in,

[ksbeattie]

@lbianchi-lbl @ksbeattie We should enable pylint checking for this ASAP to make sure it doesn't start creeping back in,

I agree, so I've raised the priority on this