VolumeFlow conversion for UkGallonsPerHour is incorrect.

[mswpearson]

Describe the bug
The defined VolumeFlow conversion value for UkGallonsPerHour is incorrect by a significant amount.

To Reproduce
N/A

Expected behavior
The conversion value is configured as 791887.667, but after a google search and some manual testing I found the conversion value should be more like 791889.294.

This is a significant difference of around 2.5 units.

Screenshots
Google suggestion:

Failing unit test:

Manual generation of the real conversion value:

Configured value in VolumeFlow.json

Additional context
I was writing unit tests to ensure the right conversions were taking place in my application.

A test for the conversion between L/s to gal (imp.)/h was failing due to the values being not quite right, even to a small degree of accuracy.

This prompted me to find the root cause of the issue which appears to be this inaccurate configured conversion value.

Funnily enough, in the original commit for this quantity the conversion number and the value used in the tests is different! Maybe I'm misunderstanding that test value though. Original commit: 4eb94e9

[lipchev]

I've also noticed some issues while doing one of my PRs targeting v6. In my change-set I've tried to resolve them by unifying the conversions using the same constant:

VolumeFlow.json:

 {
   "SingularName": "CubicFootPerSecond",
   "PluralName": "CubicFeetPerSecond",
   "FromUnitToBaseFunc": "{x} * 0.028316846592",
   "FromBaseToUnitFunc": "{x} / 0.028316846592",
   "Localization": [
     {
       "Culture": "en-US",
       "Abbreviations": [ "ft³/s" ]
     }
   ]
 },
 {
   "SingularName": "CubicFootPerMinute",
   "PluralName": "CubicFeetPerMinute",
   "FromUnitToBaseFunc": "{x} * 0.028316846592 / 60",
   "FromBaseToUnitFunc": "{x} / (0.028316846592 / 60)",
   "Localization": [
     {
       "Culture": "en-US",
       "Abbreviations": [ "ft³/min", "CFM" ]
     }
   ]
 },
 {
   "SingularName": "CubicFootPerHour",
   "PluralName": "CubicFeetPerHour",
   "FromUnitToBaseFunc": "{x} * 0.028316846592 / 3600",
   "FromBaseToUnitFunc": "{x} / (0.028316846592 / 3600)",
   "Localization": [
     {
       "Culture": "en-US",
       "Abbreviations": [ "ft³/h", "cf/hr" ]
     }
   ]
 },

StandardVolumeFlow.json:

  {
    "SingularName": "StandardCubicFootPerSecond",
    "PluralName": "StandardCubicFeetPerSecond",
    "FromUnitToBaseFunc": "{x} * 0.028316846592",
    "FromBaseToUnitFunc": "{x} / 0.028316846592",
    "Localization": [
      {
        "Culture": "en-US",
        "Abbreviations": [ "Sft³/s" ]
      }
    ]
  },
  {
    "SingularName": "StandardCubicFootPerMinute",
    "PluralName": "StandardCubicFeetPerMinute",
    "FromUnitToBaseFunc": "{x} * 0.028316846592 / 60",
    "FromBaseToUnitFunc": "{x} / (0.028316846592 / 60)",
    "Localization": [
      {
        "Culture": "en-US",
        "Abbreviations": [ "scfm" ]
      }
    ]
  },
  {
    "SingularName": "StandardCubicFootPerHour",
    "PluralName": "StandardCubicFeetPerHour",
    "FromUnitToBaseFunc": "{x} * 0.028316846592 / 3600",
    "FromBaseToUnitFunc": "{x} / (0.028316846592 / 3600)",
    "Localization": [
      {
        "Culture": "en-US",
        "Abbreviations": [ "scfh" ]
      }
    ]
  }

Perhaps we could do the same for the Uk stuff as well?

[mswpearson]

I like this approach a lot, keeps it consistent and accurate.

Do you have a PR in for this yet? If so would you be happy to make the uk fixes at the same time?

[lipchev]

Well, I was going to make one.. However there are a quite a few matches when we search for Hour so I was thinking of making one "Unifying the conversion format for all time based units".
@angularsen Do you think we should make this for v5 or v6?

[angularsen]

We can do both v5 and v6 if you are up for it 👍

[lipchev]

I've so far prepared the "correct" json files for Volume, VolumeFlow, StandardVolumeFlow, Mass, ForceChangeRate, AreaMomentOfInertia and Molarity. And I don't think I've even reached half of it yet..
So how do you want me to group the PRs? One for all or grouped somehow (like "Length", "Volume", "VolumeFlow", "StandardVolumeFlow").

I haven't had to change any of the unit tests so far, so given how I've used comment-links on each of the "basic conversion factors" (e.g. Pounds, Acres, Barrels etc.) it should be a pretty straightforward review.:

 {
   "SingularName": "BoardFoot",
   "PluralName": "BoardFeet",
   "XmlDocSummary": "The board foot or board-foot is a unit of measurement for the volume of lumber in the United States and Canada. It equals the volume of a board that is one-foot (305 mm) in length, one-foot (305 mm) in width, and one-inch (25.4 mm) in thickness.",
   "XmlDocRemarks": "https://en.wikipedia.org/wiki/Board_foot",
   "FromUnitToBaseFunc": "{x} * (0.028316846592 / 12)",
   "FromBaseToUnitFunc": "{x} / (0.028316846592 / 12)",
   "Localization": [
     {
       "Culture": "en-US",
       "Abbreviations": [ "bf", "board foot", "board feet" ]
     },
     {
       "Culture": "fr-CA",
       "Abbreviations": [ "pmp", "pied-planche", "pied de planche" ]
     }
   ]
 }

[angularsen]

I'm fine either way, no need for any particular grouping for review purposes.
But, since the change will be fairly similar across the board, then larger PRs may speed things up.
However we can merge in in smaller pieces if it makes it easier for you.

[github-actions]

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days.

[github-actions]

This issue was automatically closed due to inactivity.