BioSTEAMDevelopmentGroup / biosteam

The Biorefinery Simulation and Techno-Economic Analysis Modules; Life Cycle Assessment; Chemical Process Simulation Under Uncertainty
Other
175 stars 35 forks source link

[Critical] Bug in failed parameter setter #88

Closed yalinli2 closed 2 years ago

yalinli2 commented 2 years ago

@yoelcortes I found this hard-to-find bug while helping @shionwatabe debugging her QSDsan systems. Essentially, when setting biosteam's model parameters, failed setter functions will prevent setters for the parameters after this failed parameter from being executed, but NO error will be raised, which, depending on the position of this failed parameter in the sorted parameter set, might prevent quite a number of parameters' values from being updated, and you might not even notice it because no error is raised.

This example should explain it:

import biosteam as bst
from chaospy import distributions as shape
from biorefineries import cornstover as cs
cs.load()

model = bst.Model(cs.cornstover_sys, exception_hook='raise')

param = model.parameter

D = shape.Uniform(4, 6)
@param(name='setter1', element=cs.cornstover, kind='coupled', distribution=D)
def good_param_setter1(i):
    cs.cornstover.price = i
    print(f'Feedstock: {cs.cornstover.price}')

D = shape.Uniform(4, 6)
@param(name='setter2', element=cs.cornstover, kind='coupled', distribution=D)
def bad_param_setter(i):
    assert False # this clear won't work

D = shape.Uniform(4, 6)
@param(name='setter3', element=cs.ethanol, kind='coupled', distribution=D)
def good_param_setter2(i):
    cs.ethanol.price = i
    print(f'Ethanol: {cs.ethanol.price}')

model.metric(lambda: cs.cornstover.price, 'Cornstover price')
model.metric(lambda: cs.ethanol.price, 'Ethanol price')

samples = model.sample(N=2, rule='L')
model.load_samples(samples)
model.evaluate()

My console outputs: Screen Shot 2022-03-23 at 5 30 12 PM

No error was raised despite of that assert False, and ethanol's price is not updated

yoelcortes commented 2 years ago

Fixed! Thanks for reporting!