BioSTEAMDevelopmentGroup / thermosteam

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

Stream.proxy #46

Closed yalinli2 closed 2 years ago

yalinli2 commented 2 years ago

Hey @yoelcortes When creating a proxy (through either proxy or flow_proxy) of one stream, I noticed that the _link of the new stream is set to the original stream, while the _link of the original stream is still not changed, is this intentional? https://github.com/BioSTEAMDevelopmentGroup/thermosteam/blob/44ba1ee4616eb524d76a2e8dd7e63b027a883389/thermosteam/_stream.py#L1706

One counter-argument could be that what if the original stream itself is a proxy? I guess in that case you could set the new stream's link to the original stream's link?

Also, I think it'll be beneficial to note in the doc of proxy that the price will not be kept as the same of the time, I couldn't think of a way to keep the price synced as all times without invoking a while loop to check the _link of all streams

Happy to discuss, thanks!

yoelcortes commented 2 years ago

Hi Yalin,

I'm actually thinking about removing the _link attribute and replacing it with a new _linked attribute as a boolean denoting whether or not it is linked. The _link attribute is troublesome because of 2 reasons:

  1. Stream can be unlinked and unit operations can work without stream links. The unlinking feature is needed for handling streams with multiple phases (it is necessary to create a new array of flow rates when multiple phases are present). Many bugs in biosteam were fixed thanks to the ability to unlink.
  2. Unlinking would have to inform every other stream linked to it that they are not linked anymore. This is not possible unless we store all such streams as an attribute and run more complicated logic.

I'll probably push an update in a couple of days with this change and the proxy docstring note about not carrying the price and characterization factors (with a few other minor changes).

For purposes of keeping values (like the price) synced at all times, you can try the free_properties package:

from free_properties import PropertyFactory
import thermosteam as tmo
tmo.settings.set_thermo(['Water'])
prices= {}
@PropertyFactory(slots=['name'], units='USD/kg',)
def StreamPrice(self):
    return prices[self.name]

water_price = StreamPrice(name='water')
s_water = tmo.Stream(Water=1, units='kg/hr')
s_water_proxy = s_water.proxy()
s_water_proxy._price = s_water._price = water_price
prices['water'] = 2.
assert s_water.cost == s_water_proxy.cost

But this is a little complicated. I'd be happy to meet with you to discuss more and hopefully provide a good solution.

Thanks!

yalinli2 commented 2 years ago

Ah I see the problems, dropping the _link attribute sounds good to me.

As the price, noting in the doc is good enough, I actually think it's better to not link the price, since in quite some scenarios I just want the flow info/thermo conditions to be kept in sync, keeping price the same might do things that I don't intend to do. And it is consistent with copy_like not copying the price.

Thanks for the tip! The free_properties package is cool :)