thermotools / thermopack

Thermopack is a thermodynamic model library for fluid properties and PVT calculations
Other
51 stars 13 forks source link

New return pattern #102

Closed vegardjervell closed 10 months ago

vegardjervell commented 1 year ago

To better organise the values returned from methods that compute differentials, I've implemented the Differentials and Property structs. I've made them such that adjusting old code should be as painless as possible. In short:

Current return pattern:

mu, dmudt, dmudn = eos.chemical_potential_tv(T, V, n, dmudt=True, dmudn=True)

Using new structs we can do either

mu, dmu = eos.chemical_potential_tv(T, V, n, dmudt=True, dmudn=True)
dmudT = dmu.dT
dmudn = dmu.dn

or

mu, (dmudt, dmudn) = eos.chemical_potential_tv(T, V, n, dmudt=True, dmudn=True)

such that adapting existing code to this pattern just amounts to adding the parentheses. Additionally, where we currently have

mu, = eos.chemical_potential_tv(T, V, n) # either this
mu = eos.chemical_potential_tv(T, V, n)[0] # or this

the new structs give us

mu = eos.chemical_potential_tv(T, V, n)

such that existing code can be adapted by deleting the comma/brackets.

I've modified the makescript.py such that if you build thermopack using

python makescript.py [optim/debug] -diffs=v2

The return pattern will be completely backwards compatible.

As of now I've only made the adaptation to chemical_potential_tv, but modifying other methods is only a matter of changing the return statement to

prop = utils.Property.from_return_tuple(return_tuple, (dmudt, dmudv, dmudn), 'tvn')
return prop.unpack()

where (dmudt, dmudv, dmudn) are the flags passed into the thermopack method, and 'tvn' is swapped out for 'tpn' for methods computing stuff as functions of tpn.

I thought it would be nice with some feedback before implementing anything more :)

morteham commented 12 months ago

Looks very good! You should continue adding this for all methods.

ailoa commented 12 months ago

An excellent solution, Vegard.

vegardjervell commented 12 months ago

Thanks for nice feedback! I've now updated all relevant methods to use the new return pattern, and while I was at it I implemented a return struct for get_binary_pxy and get_binary_txy that, with the same idea as the FlashResult object, imlements __iter__ and __getitem__ to be completely backwards compatible, while giving more intuitive access to the returned variables.

Using these structs we can do either

pxy = get_binary_pxy(T)

plt.plot(pxy.lle.x1, pxy.lle.p) # Liq. 1 composition (Old : pxy[0][0], pxy[0][2])
plt.plot(pxy.lle.x2, pxy.lle.p) # Liq. 2 composition (Old: pxy[0][1], pxy[0][2])

plt.plot(pxy.l1ve.x, pxy.l1ve.p) # Liq. 1 composition (Old : pxy[1][0], pxy[1][2])
plt.plot(pxy.l1ve.y, pxy.l1ve.p) # Vapour composition (Old : pxy[1][1], pxy[1][2])

plt.plot(pxy.l2ve.x, pxy.l2ve.p) # Liq. 1 composition (Old : pxy[2][0], pxy[2][2])
plt.plot(pxy.l2ve.y, pxy.l2ve.p) # Vapour composition (Old : pxy[2][1], pxy[2][2])

or

lle, l1ve, l2ve = get_binary_pxy(T)
# Is the same as 
pxy = get_binary_pxy(T)
lle = pxy.lle
l1ve = pxy.l1ve
l2ve = pxy.l2ve

As mentioned, I've tested to make sure that you won't notice a difference if you treat this as if get_binary_pxy still returns three tuples with three arrays each.

If you build with -diffs=v2, the non-existent phase equilibria still give (None, None, None), while if you build without the flag, non-existent equilibria are ([], [], []), that is: Empty arrays. I chose the latter, because then I don't need a bunch of if ... is not None statements when I want to plot a binary-xy envelope, and am unsure about whether there is an LLE.

morteham commented 10 months ago

Looks very nice! Can you update the examples to run with the new interface, before we approve the PR?

vegardjervell commented 10 months ago

I've updated the examples now, and discovered that I had to make some changes to get_envelope_twophase and thermopack_state in order to make those compatible with both return patterns. I believe all examples / tests run as expected now, but please double check especially the last changes to thermo.py and thermopack_state.py in case I missed something that isn't tested in any examples.