chmarti1 / PYroMat

PYroMat thermodynamic properties in Python
http://pyromat.org
Other
71 stars 13 forks source link

air.T_s claims temperature out-of-bounds when using reverse values from air.s #52

Closed danieljuschus closed 1 year ago

danieljuschus commented 2 years ago
import pyromat as pm
air = pm.get("ig.air")
s = air.s(250,1)
air.T_s(s,1)

should result in 250, but raises the following error: PMParamError: All of the specified states were out-of-bounds. Legal temperatures for ig.air are between 200.0 and 6000.0 Kelvin.

jranalli commented 2 years ago

So the problem there has to do with the way PyroMat interprets arguments that aren't explicitly specified. As of ver 2.2, the first positional argument is always assumed to be T and the second p. Since T_s is a legacy function, it does retain the first argument as s, but then the next one is immediately interpreted as T. So what's really being done in your function call is: air.T_s(s=s, T=1)

I'll let @chmarti1 speak up as to how he wants to deal with the function headers on stuff like this, but you could fix this in your code right away by specifying explicitly air.T_s(s, p=1). I would suggest that it's almost always better to be explicit with the args to avoid any confusion.

Of note as well is that since 2.2, the calls like T_s aren't necessary anymore, because the underlying argument parser handles all possible arg pairs. So calling T_s is the same as calling air.T(s=s, p=1), with the latter being more straightforward and preferred at this point. It does mean updating code, but that ought to be more stable.

chmarti1 commented 2 years ago

@jranalli is right - this is a small departure from the legacy T_s() behavior. Technically, it breaks reverse compatibility in a way that is undocumented, which makes it a bug. @jranalli's fix absolutely works. Meanwhile, I'd like to push an update that reverts to the original call signatures of the T_s() and T_h() inverse methods.

In a related note, I just noticed a typo in the documentation that calls these function "Depreciated" instead of "Deprecated" (oops). The point is the same. We'll keep the T_? inverse routines for all version 2 code, but future applications should avoid them.

chmarti1 commented 2 years ago

I've looked at your code. It looks like line 117 is the only one that needs a fix to quickly avoid the issue. I recommend just changing it to read

t3_is = float(air.T(s=s1, p=p3))

That will work with version 2.2.1.

If you want, you can also abandon def p_s_theo() on line 26. PYroMat now supports p(s,T) call signatures.

Thanks for calling this to our attention!

danieljuschus commented 2 years ago

Thanks! That worked.