oie-mines-paristech / lca_algebraic

Layer over brightway2 for algebraic definition of parametric models and super fast computation of LCA
BSD 2-Clause "Simplified" License
34 stars 18 forks source link

updateExchanges() unexpected behaviour with '*' wildcard and 'old_amount' parameter #54

Closed felixpollet closed 1 month ago

felixpollet commented 1 month ago

Hello,

The method updateExchanges() of ActivityExtended has an unexpected behaviour when updating multiple exchanges using the * wildcard simultaneously with the old_amount parameter. The old amount of the first exchange overrides the formulas of the subsequent exchanges.

Here is an example :

act.updateExchanges({
    "electricity*": dict(amount=agb.old_amount, input=elec_mix)
})

What I would expect here: the same results as when calling act.updateExchanges({"electricity*": elec_mix)}). That is, all exchanges with electricity keep their respective old amount and only the input activity is modified.

What I get: All amounts are overridden by the amount of the first exchange found with electricity.

This is a basic example, but this issue prevents from updating simultaneously multiple exchanges and their amount with a formula including old_amount.

Fix for this issue: This issue is due to the attrs dict that is updated in the for exch in exchs loop. The first time attrs.update() is called, the old_amount parameter is replaced by the (float) value of the exchange. Therefore, for the subsequent exchanges, the old_amount parameter is no longer a symbol but a float, so is not updated. I propose to create a copy of the attrs dict before any update, so that each exchange is updated individually.

for exch in exchs:
    attrs_exch = attrs.copy()   # <-- this is new
    if attrs_exch is None:
        exch.delete()
        exch.save()
        continue

    # Single value ? => amount
    if not isinstance(attrs_exch, dict):
        if isinstance(attrs_exch, Activity):
            attrs_exch = dict(input=attrs_exch)
        else:
            attrs_exch = dict(amount=attrs_exch)

    if "amount" in attrs_exch:
        attrs_exch.update(_amountToFormula(attrs_exch["amount"], exch["amount"]))

    exch.update(attrs_exch)
    exch.save()
felixpollet commented 1 month ago

I created pull request #55 with the proposed modification :-)

raphaeljolivet commented 1 month ago

Hi, thanks for this contribution. I just merged it in the master branch