[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