Promotion rules

[EricForgy]

julia> one = FixedDecimal{Int,2}(1)
FixedDecimal{Int64,2}(1.00)

julia> one/3
FixedDecimal{Int64,2}(0.33)

julia> one/3.0
0.3333333333333333

Intuitively, I would expect one/3 === one/3.0 since

julia> 1/3 === 1/3.0
true

Strictly speaking, one/3 should probably also be a Float64, BUT...

I think users of this package might reasonably expect both

julia> one/3
FixedDecimal{Int64,2}(0.33)

and

julia> one/3.0
FixedDecimal{Int64,2}(0.33)

Otherwise, there would need to be a lot of conversion in the code that would kind of defeat its purpose. For example, if you wanted the result of one/3.0 to remain a FixedDecimal, you'd need to do something like

> onethird = FixedDecimal{Int,2}(one/3.0)

I think we should either:

1. Make one/3 also a Float64 so one/3 === one/3.0 or
2. Make one/3.0 also a FixedDecimal so one/3 === one/3.0

I kind of prefer the latter, but in any case it currently seems inconsistent.

What do you think?

[EricForgy]

I'll give a simplied example that I bumped into...

I am working on a financial application using FixedDecimal for payment amounts.

cost = Fixed{Int,2}(5)
sales_tax_rate = .05
sales_tax = cost*sales_tax_rate
total = cost + sale_tax

💥 I was expecting a FixedDecimal.

To fix it, I'd need to do something like

cost = Fixed{Int,2}(5)
sales_tax_rate = .05
sales_tax = FixedDecimal{Int,2}(cost*sales_tax_rate)
total = cost + sale_tax

Not the end of the world, but it feels a little clumsy to me.

[EricForgy]

Over on Slack, someone (I'll edit his name back here if he gives permission) pointed out that

if we consider each type as having a “promotiness” score, that represents how much it wants operations including it as an input to have it’s type.

Then Int < FixedPoint < Float

This makes sense and explains how

Int/FixedDecimal -> FixedDecimal, FixedDecimal/Int -> FixedDecimal

and

FixedDecimal/Float -> Float

is more consistent than it seemed to me at first.

So maybe it is ok 🤔

[EricForgy]

Maybe it is not so clear cut?

If "promotiness" score was like Int < FixedDecimal < Float, then I'd expect

FixedDecimal/FixedDecimal -> Float

but we have

julia> one = FixedDecimal{Int,2}(1)
FixedDecimal{Int64,2}(1.00)

julia> three = 3one
FixedDecimal{Int64,2}(3.00)

julia> one/three
FixedDecimal{Int64,2}(0.33)

I am still kind of leaning to having Int < Float < FixedDecimal 🤔

Edit: It would make some sense to me if

1/three === one/3 === one/3.0 === one/three 

The question is, should these all be FixedDecimal or Float64? I know my vote 😊

Edit^2: Looks like I am addressing this comment 😊

https://github.com/JuliaMath/FixedPointDecimals.jl/blob/master/src/FixedPointDecimals.jl#L333-L334

# TODO: decide if these are the right semantics;
# right now we pick the bigger int type and the bigger decimal point

[EricForgy]

Hi @NHDaly 👋

I just hopped onto the latest branch of your outstanding PR and see the following:

julia> using FixedPointDecimals

julia> one = FixedDecimal{Int,2}(1)
FixedDecimal{Int64,2}(1.00)

julia> three = 3one
FixedDecimal{Int64,2}(3.00)

julia> 1/three
FixedDecimal{Int64,2}(0.33)

julia> 1.0/three
0.3333333333333333

julia> one/3
FixedDecimal{Int64,2}(0.33)

julia> one/3.0
0.3333333333333333

julia> one/three
FixedDecimal{Int64,2}(0.33)

julia> div(one,three)
FixedDecimal{Int64,2}(0.00)

julia> 1+three
FixedDecimal{Int64,2}(4.00)

julia> 1.0+three
4.0

julia> one+three
FixedDecimal{Int64,2}(4.00)

If this is an intentional design choice that has been discussed / vetted / agreed, then no worries. I can live with it. However,

https://github.com/JuliaMath/FixedPointDecimals.jl/blob/master/src/FixedPointDecimals.jl#L333-L334

does make it sound like it is still open for discussion.

There is a case that could be made though that operations involving Float64 should not widen to Float64, but it seems almost like a judgement call.

Edit: The way it currently is still feels inconsistent to me. We either widen or we don't widen. It seems a little mixed up right now.

Here is what I would do (two options):

Option 1: Math operations widen to Float64

julia> 1/three
0.3333333333333333

julia> 1.0/three
0.3333333333333333

julia> one/3
0.3333333333333333

julia> one/3.0
0.3333333333333333

julia> one/three
0.3333333333333333

julia> div(one,three) # This seems fine
FixedDecimal{Int64,2}(0.00)

julia> 1+three
FixedDecimal{Int64,2}(4.00)

julia> 1.0+three
4.0

julia> one+three
FixedDecimal{Int64,2}(4.00)

Option 2: Math operations do not widen to Float64

julia> 1/three
FixedDecimal{Int64,2}(0.33)

julia> 1.0/three
FixedDecimal{Int64,2}(0.33)

julia> one/3
FixedDecimal{Int64,2}(0.33)

julia> one/3.0
FixedDecimal{Int64,2}(0.33)

julia> one/three
FixedDecimal{Int64,2}(0.33)

julia> div(one,three)
FixedDecimal{Int64,2}(0.00)

julia> 1+three
FixedDecimal{Int64,2}(4.00)

julia> 1.0+three
FixedDecimal{Int64,2}(4.00)

julia> one+three
FixedDecimal{Int64,2}(4.00)

I'm fine with either Option 1 or Option 2, but the current mix feels inconsistent to me.

Mathematically, Option 1 feels better, but Option 2 is also fine I think.

[NHDaly]

Hey @EricForgy, I'm glad you pointed this out! I agree it seems like probably surprising behavior, but I'm not too sure
how best to proceed either. It's certainly surprising behavior to me.

I agree with your feelings that we should think this out carefully. Thanks for raising the discussion.

I wonder why the promotiness of FixedDecimals is less than Floats? I feel like both types are used to represent fractional numbers, so I could see that argument going either way. I guess floats can represent more numbers, but at the cost of precision of course. And presumably users of FixedPointDecimals are here for the precision (for usually financial applications I think).

Since the FixedPointDecimal is a heavier-weight type, and the conversions are expensive, I think I would lean towards making it a more promoty type than Floats. I'm interested to hear what others think though! :)

(I'm on my phone now but I'll think more about this tomorrow!)

[EricForgy]

Hi @NHDaly 👋

Thanks for your comment 🙏

It sounds like we are tilting the same direction and I agree it would be good to hear the opinion of some others.

Option 1 above puts promotiness of FD < Float.

Option 2 above puts promotiness of FD > Float.

Mathematically speaking, Option 1 feels right since a FixedDecimal is more integer-like than Float, but the downside is that it makes FixedDecimal very fragile. It is very easy to inadvertently bump yourself out of FixedDecimal and into Float requiring you to frequently convert back. That is, unless you initialize all your variables, e.g. admin_fee and sales_tax_rate in my example, with FixedDecimal, but even then, I would think FD / FD -> Float is more consistent if we go with Option 1 🤔

However, this is a case where I think real-world use cases might trump mathematical correctness. In my case with financial applications, I kind of expect FD > Float so that things like my example

cost = Fixed{Int,2}(5)
sales_tax_rate = .05
sales_tax = cost*sales_tax_rate
total = cost + sale_tax

work as expected.

Then again, I'm not sure which option would be fastest. We might need to benchmark each one to see and I would go with whichever is faster because I already take care of this in my package. I have a Position type which wraps a FixedDecimal so

cost = Position(FI.USD,5)
sales_tax_rate = .05
sales_tax = cost*sales_tax_rate
total = cost + sale_tax

already works the way I want it to, but there may be some unnecessary conversion going on behind the scenes because of what is happening with FixedDecimal.

[NHDaly]

Sorry for the delay @EricForgy. Thanks again for bringing this up.

Just adding some more thoughts:

If this is an intentional design choice that has been discussed / vetted / agreed, then no worries. I can live with it. However,
https://github.com/JuliaMath/FixedPointDecimals.jl/blob/master/src/FixedPointDecimals.jl#L333-L334
does make it sound like it is still open for discussion.

I'm not sure if this design choice has been discussed / vetted / agreed. And if it has been, it's been a while! 😋 It's probably worth pointing out, though, that the comment you're specifically referencing there is, I think, instead about what to do if you multiply FD{Int64,2} and FD{Int32,4}. Right now the output would be FD{Int64,4}.

But I agree with you that the comment implies that maybe a lot of this hasn't been too firmly set in stone.

Another point to consider is ÷. I changed this in #40 to return an FD, but before it was returning an Int. That seemed weird to me, but now I'm less sure. Maybe it is better to stay consistent with Rational?:

julia> FixedDecimal{Int64,4}(3) ÷ 2.0
1.0

julia> 3//2 ÷ 2//2
1

Now it will be more in line with the Int < FD < Float promotiness, i think, but worth considering for how we move forward. (And regarding performance, in this case converting back to an FD actually is slightly slower, so maybe making that change was a mistake, I'm not sure. Maybe just picking the most performant path for everything is the best approach?)

---

Anyway, I agree that it would be nice to have some clarity here. It currently seems like we aren't following any consistent rules, and at least identifying and following some rule would be good. Ideally, the simpler the rule the better.

My vote would be for Option 2, everything promotes to an FD, but I'd be happy with anything that's easy to understand and self-consistent.

[EricForgy]

Hi @NHDaly 👋

I had completely forgotten about this 😅

Another point to consider is ÷. I changed this in #40 to return an FD, but before it was returning an Int. That seemed weird to me, but now I'm less sure. Maybe it is better to stay consistent with Rational?

I have always thought of ÷ as "integer division" so it makes sense to me it would return Int, so I would probably stick with that here as well. As, from the docs:

> help?> div
search: div divrem DivideError splitdrive code_native @code_native

  div(x, y)
  ÷(x, y)

  The quotient from Euclidean division. Computes x/y, truncated to an integer.

indicates it should return an Int.

My vote would be for Option 2, everything promotes to an FD, but I'd be happy with anything that's easy to understand and self-consistent.

That would be my vote as well. Do you know of anyone else using this package? It would be good to get their opinions.

Actually, this seems obvious to say it, but what if we just completely mimick the promotiness of Rational. It seems to strike a nice balance. Well, I just checked again and aside from div, FD does match the promtiness of Rational so after all this noise, I think it is probably ok as it is? But not sure 😅

Edit: I dunno. Even after discovering the match with Rational, I still lean toward Option 2. I am not sure Rational is used for anything practical, whereas FD will be used in financial applications, where the expectations might be different 🤔