[PATCH 0/3] Some cylinder pressure handling cleanups
Dirk Hohndel
dirk at hohndel.org
Wed Feb 24 15:45:25 PST 2016
On Wed, Feb 24, 2016 at 01:26:30PM -0800, Linus Torvalds wrote:
>
> The first patch in the series doesn't actually change anything, it just
> makes sure a function that will be changed is private, and removes another
> one that isn't actually used and is questionable to begin with. So that
> patch is a no-brainer, I think.
Yep, already applied
> The second patch came about from the noticing that our
> "match_standard_cylinder()" logic actually tried to take the
> compressibility of gases into account. That happened to not matter for
> 2400 and 3000 psi cylinders, because our old gas compressibility
> heuristics made the differences there unnoticeable, but if we change our
> compressibility thing to give the full 3% change at 3000 psi, then that is
> a difference of a couple of cuft.
>
> There is actually a theoretical argument for why the nominal imperial
> cylinder sizes should *not* take compressibility into account: it depends
> on the gas in question. The compressiblity of oxygen is noticeably
> different from that of air, for example.
>
> So from an imperial cylinder naming perspective, it actually kind of makes
> sense to say "AL80 means 80 cuft of _ideal_ gas at STP".
>
> It is also worth noting that the *other* places where we create cylinder
> sizes (notably get_volume_string()) are not using the compressibility
> factor, so the change in patch #2 really is mostly about being internally
> consistent (but also about getting rid of uses of that odd "surface
> factor", which is how I noticed this in the first place).
Yes, I consider this more consistent and following the principle of least
surprise. So I took this as well.
> The third patch is the one that gets rid of our current slightly hacky
> surface_volume_multiplier(), which is now no longer used anywhere else but
> by the "gas_volume()" helper.
>
> Instead of that surface_volume_multiplier(), it introduces the notion of
> compressibility Z factor and makes gas_volume() use that instead for its
> calculations, and a function to calculate it (called, not surprisingly,
> "gas_compressibility_factor()").
>
> NOTE! That function takes a gasmix, but then ignores the hell out of it,
> and just uses the Wikipedia table for air at 300K.
:-)
> That third one should be looked at critically. I got the linear
> interpolation wrong (mixed up "frac" and "1-frac") in the first version,
> and wouldn't have noticed at all if I hadn't done some testing with hacky
> debug code. The compressibility factor isn't really noticeable in any
> normal way.
I commented on that patch, not beause I think your math is wrong but
because I'm suspicious of the table that you started with...
/D
More information about the subsurface
mailing list