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 _______________________________________________ subsurface mailing list [email protected] http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
