elixir-cldr / cldr_units

Unit formatting (volume, area, length, ...) functions for the Common Locale Data Repository (CLDR)
Other
16 stars 13 forks source link

Multiplication of `per` units #32

Open maennchen opened 1 year ago

maennchen commented 1 year ago

It would be nice if composite (per) units could be multiplied with its base units.

For example:

drain_rate = Cldr.Unit.new!("liter_per_hour", 6)
time = Cldr.Unit.new!("minute", 30)
Cldr.Unit.mult(drain_rate, time)
# => #Cldr.Unit<"liter", 3>
kipcole9 commented 1 year ago

Great suggestion. So basically allow "per" unit to be multiplied by either its numerator base unit or its denominator base unit since these are algebraically straight forward.

That part of the code is somewhat complex so it might take me a little while to implement but I should have it done by the end of the weekend.

maennchen commented 1 year ago

Yes, exactly that.

This was just something like I stumbled upon when trying to refactor some code. I have a working solution right now, so this is not at all something pressing. Just a good idea I think.

Take your time and let me know if I can be of help 😊

kipcole9 commented 1 year ago

Not forgotten, just a bit time-challenged a the moment.

kipcole9 commented 1 year ago

Back on this issue this week now that I've cleared some backlog items. Will try hard to have this resolved by weeks end.

kipcole9 commented 1 year ago

I've pushed two commits that have an initial implementation:

The work is not complete however. Using your example:

iex> drain_rate = Cldr.Unit.new!("liter_per_hour", 6)
Cldr.Unit.new!("liter_per_hour", 6)
iex> time = Cldr.Unit.new!("minute", 30)
Cldr.Unit.new!(:minute, 30)
iex> Cldr.Unit.mult(drain_rate, time)
Cldr.Unit.new!("liter_minute_per_hour", 180)

You can see that the factors aren't being reduced. I have an implementation to reduce factors but it resulted in a quite large number of test failures on the standard CLDR test data suite so I need to revisit that part next.

Comments and suggestions are of course welcome.

maennchen commented 1 year ago

@kipcole9 That is awesome. :heart:

So the missing piece if I understand correctly is that we have to identify if there are compatible units in the dividend and the divisor. If there are, we eliminate them from both.

Therefore $\frac{6\text{ liter}\cdot30\text{ minute}}{1\text{ hour}}$ would be simplified to $6\text{ liter}\cdot\frac{30\text {minute}}{1\text{ hour}}$, $6\text{ liter}\cdot\frac{30\text {minute}}{60\text{ minute}}$, $6\text{ liter}\cdot\frac{1}{2}$ and then to $3\text{ liter}$.

How do we represent divisions that do not work clean? Just as a ratio?

drain_rate = Cldr.Unit.new!("liter_per_hour", 1)
time = Cldr.Unit.new!("minute", 17)
Cldr.Unit.mult(drain_rate, time)
# => Cldr.Unit.new!("liter", 60 <|> 17)

We could probably approach it something like this (pseudo-code):

simplified_units = for %Cldr.Unit{amount = dividend_amount, unit: dividend_unit} = dividend <- dividends(input),
  divisor <- divisors(input),
  Cldr.Unit.compatible?(dividend, divisor),
  # Careful, this pseudo code ignores that each divisor / dividend must only be used once...
  reduce: input do
  acc ->
    %Cldr.Unit{amount = divisor_amount} = Cldr.Unit.convert!(divisor, dividend_unit)

    acc
    |> remove_dividend(dividend_unit)
    |> remove_divisor(divisor_unit)
    |> mult(dividend_amount <|> divisor_amount)
end

# Cldr.Unit.new!(x, 7 <|> 1) => Cldr.Unit.new!(x, 7)
simplified = case simplified_units do
  %Cldr.Unit{value: %Ratio{denominator: 1, numerator: value}} -> %Cldr.Unit{simplified_units | value: value}}
  _other -> simplified_units
end

The same applies to multiplications as well I guess. For example $6\text{ meter}\cdot50\text{ centimeter}$ should probably result in $3\text{ square meter}$.

In extreme cases we also might have $\frac{6\text{ square meter}}{3\text{ meter}}$, which should result in $2\text{ meter}$.

maennchen commented 1 year ago

Thinking about it a bit more, we probably have to take a less naive approach to solve those.

GNU Units does what we're looking for:

units "8meter/second * 300second * 5meter / 10 centimeter"
# => 120000 m

Maybe we could get some inspiration from there...

kipcole9 commented 1 year ago

Very doable. I have most of this implemented but not wired up (due to the unexplained test errors). I can wire it up for only this mult/div use case and see where that gets us. I can do some more work on this tonight my time (back in Singapore). Great hint about units, I always forget about that.

kipcole9 commented 1 year ago

And I probably should built a units semi-clone as a good test harness if nothing else.

kipcole9 commented 1 year ago

Apologies for taking so long on this. The cycle for CLDR 43 updates was longer than I planned or expected. I am nearly finished now on adding localized verified routes to ex_cldr_routes and getting back to this issue is next on the list.

bolte-17 commented 1 year ago

This seems like it's the same root problem I immediately ran across when testing out this library.

First thing I tried out was some very basic multiplications/divisions and the results were both extremely surprising and made the library not useful for my intended purpose.

Basic examples were (1 :meter) * (1 :meter) returns 1 :meter rather than 1 :meter^2 as expected. (1 :meter) / (1 :meter) returns 1 :meter rather than 1 as expected.

The docs might be implying there's a difference between div!/2 and div/2 here in that the first always returns the first argument's unit, but I'm not sure that's intended. Behavior doesn't seem to differ in that regard between the two functions.

I'm slightly confused if there's even a means to represent dimensionless quantities in this library. The configuration of new units seems to imply :unit is a thing but that's not a valid atom for Unit.new. I'd expect it to be valid to div and mult a dimensioned unit with a scalar, i.e. (5 meters) * 10 => (50 meters).

kipcole9 commented 1 year ago

All good points. This library started primary to provide unit localisation and conversion. And then I started work on basic math and algebra which clearly isn't correct of complete. And the last major refactor for the lib - which focused on conversion precision - drained me. Its time I got back to this issue and addressing your comments.

Your reigniting this issue is good motivation for me so hopefully you might care to collaborate and keep pushing. And if you can explain your use case and requirements I can keep focused on what you need for the next iteration.

bolte-17 commented 1 year ago

If you're feeling motivated that's great, but there's no rush. I was looking around for libraries mostly to put some guardrails on a bunch of durations we're currently passing around as just raw numbers, sometimes representing hours, sometimes representing seconds, etc. We also have some domain-specific units of time like treating 15 minutes as a single "unit". It looked like I could try this library out for that purpose, since it supported custom units in a way that I wouldn't have to redefine every single time unit conversion manually.

Was thinking of doing things like "round to nearest 15 minutes", with the 15 minutes being somewhat configurable, but taking an actual quantity with units rather than just an integer and then having to guess whether it was a number of hours or seconds. Could implement as something like

def nearest_multiple(quantity, unit) do
  quantity |> div(unit) |> round() |> mult(unit)
end

This library might not end up being the right tool for the job anyway- I see now how this is more focused on localization- but it seemed like those mult/div results were so surprising I should at least put out an issue or comment.

kipcole9 commented 4 months ago

It's been nearly a year but I'm back on this issue. I've had to do quite a lot of refactoring for other CLDR-related reasons but thats now done. I've added support for multiplying/dividing a unit by a scalar value (phase 1). Now I can work on multiplying and dividing "incompatible" units and then doing common factor reduction.

I'm aiming to get this done in time for a release cycle in April that coincides with CLDR 45.