search

BioSTEAMDevelopmentGroup / thermosteam

BioSTEAM's Premier Thermodynamic Engine
Other
57 stars 12 forks source link

Aliases in `Chemical.copy` #55

Closed yalinli2 closed 2 years ago

yalinli2 commented 2 years ago

The copy method basically copy everything from one chemical to another, but this could cause problem for aliases setting as it bypasses the duplicate alias check.

For example, in the corn biorefinery, Starch (and also Fiber, SolubleProtein, InsolubleProtein) is copied from Cellulose, and they end up having the same aliases https://github.com/BioSTEAMDevelopmentGroup/Bioindustrial-Park/blob/1d79328e2cb60ef15392dce2ac0134c374980460/BioSTEAM%202.x.x/biorefineries/corn/_chemicals.py#L22

>>> from biorefineries import corn as cn
>>> chems = cn.create_chemicals()
>>> chems.Cellulose.aliases
{'C6H10O5'}
>>> chems.Starch.aliases
{'C6H10O5'}

I'm OK with just fixing it in the biorefinery setting like https://github.com/BioSTEAMDevelopmentGroup/Bioindustrial-Park/blob/86fa9bc440d0e7d55a637bdcfb9a91c909928c07/BioSTEAM%202.x.x/biorefineries/corn/_chemicals.py#L22

but wondering if it's better to do something on the method, thanks!

yoelcortes commented 2 years ago

Hi Yalin,

This reply comes a little late, sorry about that! I think your concern makes sense. I removed aliases for copied chemicals. Now we have the following:

>>> from biorefineries import corn as cn
>>> chems = cn.create_chemicals()
>>> chems.Cellulose.aliases
{'C6H10O5'}
>>> chems.Starch.aliases
set()

Hope this helps, Thanks!

yalinli2 commented 2 years ago

Wonderful, thanks @yoelcortes!