Skip to content

Add VolumePerLength #604

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Mar 8, 2019
Merged

Add VolumePerLength #604

merged 8 commits into from
Mar 8, 2019

Conversation

adelmoral
Copy link
Contributor

The capacity of drill string is an essential issue in oil well control. The capacity of drillpipe, drill collars or hole is the volume of fluid that can be contained within them. (https://en.wikipedia.org/wiki/Oil_well_control#Capacity)

The units for drill string capacity are bbl/ft or l/m.

@angularsen
Copy link
Owner

angularsen commented Feb 7, 2019

Hi and thanks for contributing!

I'm a little on the fence on adding too domain specific quantities though, to avoid blowing up the size of this library. We don't yet have a good story for tackling that. Some ideas have been floating on having separate nugets for it, but there is the issue of overlap between domains and no good way to have statically typed conversions between those domains. Another option is plugging in quantities and conversions at runtime, but that story is not yet fleshed out.

What do you think @tmilnthorp ?

Common enough to include?
If not, accept now and tackle it later or postpone until until we have a good strategy for less common quantities?

@tmilnthorp
Copy link
Collaborator

Good question. Regardless though I would not make it a specific quantity like this. DrillStringCapacity is not really a quantity, but volume per length is. If we included it, I'd recommend creating a VolumePerLength quantity that supports barrels per foot, liters per meter, gallons per mile, etc.

Also this calculation is specifically for barrels per foot for a cylindrical object. You can have cubic inches per foot of square pipe for example. Or cubic feet per foot of coiled air hose (need hose diameter, coil diameter, coil spacing, etc. to figure out). Some of those conversions get very specific.

Regardless though, it just boils down to calculating how much volume something has based on the geometry of its container.

@adelmoral
Copy link
Contributor Author

adelmoral commented Feb 7, 2019

What would be required in order for a VolumePerLength quantity to be accepted to merge in? I could make the necessary changes. My team particularly needs the calculation for a cylindrical object.

@tmilnthorp
Copy link
Collaborator

While we think about it, are you able to create your own calculation methods?

public static Volume CalculateInnerCapacity( Length innerDiameter, Length length )
{
	var bblFt = Math.Pow( innerDiameter.Inches, 2.0 ) / 1029.4;
	return Volume.FromOilBarrels( bblFt / length.Feet );
}

@adelmoral
Copy link
Contributor Author

We'll look into it. We wanted to avoid handling conversions from bbl/ft to l/m and back in our code, but could do it for now. Keep me posted about the VolumePerLength quantity and I'll make the changes when you have made a decision. Thanks!

@angularsen
Copy link
Owner

angularsen commented Feb 15, 2019

DrillStringCapacity is not really a quantity, but volume per length is. If we included it, I'd recommend creating a VolumePerLength quantity that supports barrels per foot, liters per meter, gallons per mile, etc.

I was thinking the same thing. It's almost as if adding a new quantity HappyDistance with units meters, centimeters etc. It's pretty much the same thing as Length, just more happy.

VolumePerLength is dimensionally equivalent to Area (Length*Length). I guess the only reason we can't build on top of that is that we can't represent units like bbl/ft as it would be reduced to units like ft², since bbl/ft would be a strange unit to add to Area I think.

At least I can find some hits on google on volume per length all related to pipes, so I think that would be more generic and usable to a wider audience:

https://www.vici-jour.com/tubing/conv_ch.php
https://www.inchcalculator.com/pipe-volume-calculator/

I'm good with adding VolumePerLength, how about you @tmilnthorp ?

@adelmoral Adding a new quantity is not too complicated, we have detailed steps here:
https://github.com/angularsen/UnitsNet/wiki/Adding-a-New-Unit

I'm happy to assist if you have any questions on the process.

@angularsen
Copy link
Owner

I forgot this was a PR and not an issue, you have already followed the wiki steps, so what I believe remains is to:

  1. Rename the file and quantity name from DrillStringCapacity to VolumePerLength.
  2. Change BaseUnit to be closer to SI units. Would it maybe make sense to add m³/m for completeness sake? If not, we can just use LiterPerMeter.
  3. Update xmldoc

If you want to add extra methods beyond what is generated, you put those in .extra.cs files.

For example:
CustomCode\VolumePerLength.extra.cs
static VolumePerLength VolumePerLength.From(Volume volume, Length length)
static VolumePerLength VolumePerLength.From(Area a)

CustomCode\Volume.extra.cs
static Volume FromTube(Length innerDiameter, Length length)

The convention is that the result type dictates what file to put it in.

@angularsen
Copy link
Owner

@adelmoral Did you read my last feedback on how to move this PR along? Did it make sense to you? Let me know if anything is unclear.

@adelmoral
Copy link
Contributor Author

adelmoral commented Mar 6, 2019

Apologies for the late reply, it has been a busy past couple of weeks. I'll go ahead and make the changes above ASAP.

@angularsen angularsen merged commit 1edfef9 into angularsen:master Mar 8, 2019
@angularsen angularsen changed the title Adding Drill String Capacity Add VolumePerLength Mar 8, 2019
@angularsen
Copy link
Owner

Nuget on the way out.
Release UnitsNet/4.14.0 · angularsen/UnitsNet

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants