Adding heat flux units with tests #431

tmilnthorp · 2018-04-18T13:48:45Z

Adding heat flux. My first contribution so I hope everything is in order. Thank you!

angularsen

Good job adding all these units!
I found some things you can take a look at.

angularsen · 2018-04-20T18:55:20Z

UnitsNet.Tests/CustomCode/HeatFluxTests.cs

+{
+    public class HeatFluxTests : HeatFluxTestsBase
+    {
+        protected override double BtuPerSquareFootHourInOneWattPerSquareMeter => 3.16998331e-1;


I think your BTU units are what is called BTU (IT)?
We've seen those before, not sure what IT stands for. International?

For BTU I get 3.15459 (rounded):
https://www.theunitconverter.com/btu-hour-square-foot-to-watt-square-meter-conversion/

For BTU (IT) I get 0.316998331, same as you:
http://www.unitconversion.org/heat-flux-density/watts-per-square-meter-to-btus-it--per-hour-per-square-foot-conversion.html

There is also BTU (th): 0.317210471
Here is a pretty complete table:
http://www.unitconversion.org/unit_converter/heat-flux-density-ex.html

Both links use the naming BTU/Hour Square Foot and BTU/h∙ft², which would translate to BtuPerHourSquareFoot. Would that be a more widely used naming than BtuPerSquareFootHour?

That is correct, this is the international table BTU, not thermochemical. Do you distinguish anywhere? I did not see any examples. I would generally prefer to stick with it. To quote the American Physical Society:

The International Table (IT) calorie has been adopted in the publications of the Energy Information Administration of the U.S. Department of Energy (DOE/EIA) (3) and of the International Energy Agency of the Organization for Economic Co-operation and Development (OECD/IEA) (4). In view of the importance of these publications, it is reasonable to view the IT calorie as being the preferred unit for discussions of energy production and use, but there is no universally adopted practice (see also the discussion of Btu, below).

I think you are correct about the ordering with names. I will change this.

I'm no domain expert and not really familiar with these IT and Th variants myself.
We don't have a guideline on these yet, but here is what we have so far:

SpecificEnergy.CaloriePerGram

SpecificEntropy.CaloriePerGramKelvin

Energy.Calorie

Energy.BritishThermalUnit

So we don't already specify variant for calories or BTU. Not sure if these are IT or Th, or what is the most widely used. I don't find any mention of IT and Th in existing units, so I may have been confused by the Therm unit with abbreviation th.

I recall these after some discussions and googling in earlier issues, but searching I only found these so I guess we haven't thought too much about it yet:
#322 (comment) (SpecificEntropy)
#85 (BTU)

So. If I am to conclude on anything => no, we don't currently specify IT or Th and we don't distinguish them. The question is, should we? Would you know what makes the most sense?

I have not distinguished, but I can't say that is true for everyone. IT seems to be pretty standard.

Since it seems we have been using IT before and you also find that the most natural, I suggest we stick with it until someone comes around and actually needs the Th variant and a way to distinguish them.

angularsen · 2018-04-20T19:02:01Z

UnitsNet.Tests/CustomCode/HeatFluxTests.cs

+        protected override double BtuPerSquareFootMinuteInOneWattPerSquareMeter => 5.28330551e-3;
+        protected override double BtuPerSquareFootSecondInOneWattPerSquareMeter => 8.80550918e-5;
+        protected override double BtuPerSquareInchSecondInOneWattPerSquareMeter => 6.11493693e-7;
+        protected override double CalPerSquareCentiMeterSecondInOneWattPerSquareMeter => 2.39005736e-5;


I think I prefer typing these fully out, Calories and Kilocalories.

And again, you have two variants Calorie (IT) and Calorie (th).

No idea what they are, but they keep showing up:
http://www.unitconversion.org/unit_converter/heat-flux-density-ex.html

angularsen · 2018-04-20T19:10:30Z

UnitsNet/UnitDefinitions/HeatFlux.json

+      "Localization": [
+        {
+          "Culture": "en-US",
+          "Abbreviations": [ "BTU/in²/s" ]


Would BtuPerSecondSquareInch and BTU/(s∙in²) be better? Ref comments above about naming.
Same for units below.

Yes, I will change this.

angularsen · 2018-04-20T19:12:06Z

UnitsNet/UnitDefinitions/HeatFlux.json

+      ]
+    },
+    {
+      "SingularName": "KiloCalPerSquareMeterHour",


I think you can remove KiloCalPerSquareMeterHour, since you already have a prefix Kilo in CalPerSquareCentiMeterSecond?

Wouldn't the Kilo prefix only apply to CalPerSquareCentiMeterSecond? [CalPerSquareCentiMeterSecond and KiloCalPerSquareCentiMeterSecond].

This case is m²/hr, not centimeter as above.

Ah I misread. But you can change this to CalPerSquareMeterHour and add Kilo as prefix to get both Cal and Kilocal, if that makes sense to you.

If you don't need that, then keep this and fix the case-typo KiloCal -> Kilocal.

It isn't very common so I just fixed the case

angularsen

Comments on latest changes

angularsen · 2018-04-20T21:49:10Z

UnitsNet.Tests/CustomCode/HeatFluxTests.cs

+        protected override double BtuPerHourSquareFootInOneWattPerSquareMeter => 3.16998331e-1;
+        protected override double BtuPerMinuteSquareFootInOneWattPerSquareMeter => 5.28330551e-3;
+        protected override double BtuPerSecondSquareFootInOneWattPerSquareMeter => 8.80550918e-5;
+        protected override double BtuPerSecondSquareInchInOneWattPerSquareMeter => 6.11493693e-7;


I can't seem to locate a source to verify this one, but my attempt on the calculator based on BtuPerSecondSquareFootInOneWattPerSquareMeter gives me the same.

I checked Google and WolframAlpha

angularsen · 2018-04-20T21:50:16Z

UnitsNet.Tests/CustomCode/HeatFluxTests.cs

+        protected override double BtuPerMinuteSquareFootInOneWattPerSquareMeter => 5.28330551e-3;
+        protected override double BtuPerSecondSquareFootInOneWattPerSquareMeter => 8.80550918e-5;
+        protected override double BtuPerSecondSquareInchInOneWattPerSquareMeter => 6.11493693e-7;
+        protected override double CalPerSecondSquareCentimeterInOneWattPerSquareMeter => 2.39005736e-5;


Will you rename all Cal to Calorie as well? For consistency with existing units.

Absolutely. Consistency is good!

angularsen · 2018-04-20T21:57:15Z

UnitsNet.Tests/CustomCode/HeatFluxTests.cs

+        protected override double CentiwattsPerSquareMeterInOneWattPerSquareMeter => 1e2;
+        protected override double DeciwattsPerSquareMeterInOneWattPerSquareMeter => 1e1;
+        protected override double KilocalPerSecondSquareCentimeterInOneWattPerSquareMeter => 2.39005736e-8;
+        protected override double KilocalPerHourSquareMeterInOneWattPerSquareMeter => 8.6042065e-1;


I'm getting 0.85985 here (doesn't say, but assume it is IT calorie) and 0.859845228 here (IT calorie).

Looks like I made the mistake here. I used thermochemical not IT :)

Fix and verify: WolframAlpha

angularsen · 2018-04-20T22:02:38Z

UnitsNet/UnitDefinitions/HeatFlux.json

      "FromUnitToBaseFunc": "x*1.16222222",
      "FromBaseToUnitFunc": "x/1.16222222",
      "Localization": [
        {
          "Culture": "en-US",
-          "Abbreviations": [ "kcal/m²/hr" ]
+          "Abbreviations": [ "kcal/hr·m²" ]


You need to add parantheses here and for the above units, like: kcal/(hr·m²)

I have never seen it written this way. Order of operations should already take care of it, no? https://www.pentairthermal.com/Images/EN-IndustrialHeatTracingUnitConversionTable-AR-H57432_tcm432-26156.pdf

Hm, good point. I'm not really sure what are the conventions on this, but I too have seen the form without the parantheses. I thought precedence was not universal for math, but rather chosen and defined per programming language.

https://en.wikipedia.org/wiki/Order_of_operations
Here it seems that division by x is treated just like an multiplication of 1/x, and if I got that right then kcal/hr·m² becomes kcal*(1/hr)*m² - which is not correct.

https://math.berkeley.edu/~gbergman/misc/numbers/ord_ops.html
Here it seems there is no universally agreed upon order, so use parantheses instead.

Do you know if there is any litterature backing up the form of not using parantheses and still being unambiguous? I'm not familiar in this territory.

tmilnthorp · 2018-04-23T13:59:11Z

UnitsNet/UnitDefinitions/HeatFlux.json

+      "Localization": [
+        {
+          "Culture": "en-US",
+          "Abbreviations": [ "BTU/hr·ft²" ]


Now I look at the others, I suspect I should abbreviate hr to h, correct?

Yes we have a guideline to use h, unless it would go against a widely used convention.

Link to guideline: https://github.com/angularsen/UnitsNet/blob/master/CONTRIBUTING.md#unit-abbreviations

tmilnthorp · 2018-04-23T14:01:29Z

UnitsNet/UnitDefinitions/HeatFlux.json

      "FromUnitToBaseFunc": "x*1.16222222",
      "FromBaseToUnitFunc": "x/1.16222222",
      "Localization": [
        {
          "Culture": "en-US",
-          "Abbreviations": [ "kcal/m²/hr" ]
+          "Abbreviations": [ "kcal/hr·m²" ]


I have never seen it written this way. Order of operations should already take care of it, no? https://www.pentairthermal.com/Images/EN-IndustrialHeatTracingUnitConversionTable-AR-H57432_tcm432-26156.pdf

tmilnthorp · 2018-04-23T15:11:52Z

I saw some other ideas floated around about base dimensions. They would be very useful. I thought about adding the following, even if not used yet:

{
  "Name": "HeatFlux",
  "BaseUnit": "WattPerSquareMeter",
  "XmlDoc": "Heat flux is the flow of energy per unit of area per unit of time",
  "BaseUnitDimensions":
  {
    "Length": 0,
    "Mass": 1,
    "Time": -3,
    "ElectricCurrent": 0,
    "ThermodynamicTemperature": 0,
    "AmountOfSubstance": 0,
    "LuminousIntensity": 0
  }
}

angularsen · 2018-04-23T17:25:12Z

Good thinking, if you didn't see it @eriove already started some work on this but it's been dormant for some time now: #275

Maybe you want to continue that work?

tmilnthorp · 2018-04-23T17:40:38Z

I'd love to. Thanks for pointing me to it. Would you be opposed to adding this info to the json files even if not yet currently used? I decided on the format above vs the si array as it is a bit more explicit.

angularsen · 2018-04-23T17:56:36Z

I wouldn't want to add it before it is actually used, no. I don't want to risk not completing the feature and having stuff in the JSON that is not in use, and confuse the next guy reading it.
As for array vs json properties, I have no particular preference as of now. I see advantages either way.

tmilnthorp · 2018-04-23T19:56:29Z

Sounds good. Thanks!

Is there anything else for this pull request?

angularsen

A few more :-)

angularsen · 2018-04-23T20:04:41Z

UnitsNet/GeneratedCode/UnitSystem.Default.g.cs

+                        new CulturesForEnumValue((int) HeatFluxUnit.BtuPerSecondSquareFoot,
+                            new[]
+                            {
+                                new AbbreviationsForCulture("en-US", "BTU/s·ft²"),


We need to nail this form first. I personally find it ambiguous the way it reads now, but I am no domain expert on this.

A quick google tells me that the these forms are used:

Btu (IT)/s/ft²

Btu/(s.ft2)

Btu (IT)/second/ft²

Btuth·in/(h·ft2·°F) (just consider the form here)

So from this, I conclude that we need to use parantheses or use a second division. What do you think?

I can point you to one of my company's product offerings, ANSYS Mechanical. It is widely used by engineers and uses the BTU/s·in² format. I again expect this due to order of operations and it's the way I am used to seeing heat flux expressed.

Alright, I'm sold :-) We will keep this form then since at least in the context of these units they seem commonly used.

Sounds good.

angularsen · 2018-04-23T20:05:51Z

UnitsNet/UnitDefinitions/HeatFlux.json

+    },
+    {
+      "SingularName": "KilocaloriePerHourSquareMeter",
+      "PluralName": "KilocaloriePerHourSquareMeter",


Plural: Kilocalories

angularsen · 2018-04-23T20:06:07Z

UnitsNet/UnitDefinitions/HeatFlux.json

+    },
+    {
+      "SingularName": "CaloriePerSecondSquareCentimeter",
+      "PluralName": "CaloriePerSecondSquareCentimeter",


Plural: Calories

angularsen · 2018-04-23T20:06:16Z

UnitsNet/UnitDefinitions/HeatFlux.json

+    },
+    {
+      "SingularName": "BtuPerHourSquareFoot",
+      "PluralName": "BtuPerHourSquareFoot",


Plural: Btus

angularsen · 2018-04-23T20:06:33Z

UnitsNet/UnitDefinitions/HeatFlux.json

+    },
+    {
+      "SingularName": "BtuPerSecondSquareFoot",
+      "PluralName": "BtuPerSecondSquareFoot",


Plural: Btus

angularsen · 2018-04-23T20:06:37Z

UnitsNet/UnitDefinitions/HeatFlux.json

+    },
+    {
+      "SingularName": "BtuPerSecondSquareInch",
+      "PluralName": "BtuPerSecondSquareInch",


Plural: Btus

angularsen · 2018-04-23T20:06:50Z

UnitsNet/UnitDefinitions/HeatFlux.json

+    },
+    {
+      "SingularName": "WattPerSquareFoot",
+      "PluralName": "WattPerSquareFoot",


Plural: Watts

Do you expect BaseUnit to also be WattsPerSquareMeter? I used WattPerSquareMeter based on what I saw in Irradiance

Use SingularName for BaseUnit. ;)

BaseUnit needs to be singular and match exactly one of the SingularName entries in the JSON, plural is only used to generate proper names of methods and properties, such as double val = myLength.Meters and double val = myHeatFlux.WattsPerSquareFoot.

Excellent, thanks for the info. Good to get acquainted as I plan to contribute in the future :)

angularsen · 2018-04-23T20:06:55Z

UnitsNet/UnitDefinitions/HeatFlux.json

+    },
+    {
+      "SingularName": "WattPerSquareInch",
+      "PluralName": "WattPerSquareInch",


Plural: Watts

tmilnthorp · 2018-04-25T20:32:06Z

Hope everything is fixed up now!

angularsen

Looks good to me!
Good job on this 😃

angularsen · 2018-04-25T22:02:58Z

Nuget 3.97 is on the way out 👍

tmilnthorp · 2018-04-26T15:33:29Z

Thanks!

tmilnthorp added 2 commits April 18, 2018 09:46

Adding heat flux units with tests

8140b11

Adding all permutations for operator overloads with heat flux

1404aaf

angularsen requested changes Apr 20, 2018

View reviewed changes

Naming fixes

23575a3

angularsen requested changes Apr 20, 2018

View reviewed changes

tmilnthorp commented Apr 23, 2018

View reviewed changes

Consistency and verification (IT) updates

531dfad

angularsen requested changes Apr 23, 2018

View reviewed changes

Fixing plural names

596ea01

angularsen approved these changes Apr 25, 2018

View reviewed changes

angularsen merged commit bad1e53 into angularsen:master Apr 25, 2018

Adding heat flux units with tests #431

Adding heat flux units with tests #431

Conversation

tmilnthorp commented Apr 18, 2018

angularsen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

angularsen Apr 20, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

angularsen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tmilnthorp Apr 23, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tmilnthorp commented Apr 23, 2018

angularsen commented Apr 23, 2018

tmilnthorp commented Apr 23, 2018

angularsen commented Apr 23, 2018

tmilnthorp commented Apr 23, 2018

angularsen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

angularsen Apr 23, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tmilnthorp commented Apr 25, 2018

angularsen left a comment

Choose a reason for hiding this comment

angularsen commented Apr 25, 2018

tmilnthorp commented Apr 26, 2018

angularsen Apr 20, 2018 •

edited

Loading

tmilnthorp Apr 23, 2018 •

edited

Loading

angularsen Apr 23, 2018 •

edited

Loading