Switch PluralRules to use FixedDecimal

[zbraniecki]

At landing time PluralRules create operands from primitive types. We'd like to switch that to support FixedDecimal.

[zbraniecki]

@sffc how do you envision this working?

FixedDecimal accepts u*/i* and PluralRules accepts Into<PluralOperands>, should we have Into<PluralOperands> for FixedDecimal?

[sffc]

I envision an impl From<&FixedDecimal> for PluralOperands, because there's no reason that the FixedDecimal needs to be consumed.

To implement the conversion, you'd use digit_at(x) for x from 0 to the most significant digit to construct i; digit_at(x) for x from the least significant digit to -1 to construct f; and so forth.

[filmil]

Is there a reason to remove support for From<primitive_type>?

[zbraniecki]

I don't think so.

[zbraniecki]

we can compare performance and if the primitive -> FixedDecimal -> PluralOperands is not a huge cost compared to primitive -> PluralOperands then we can remove. For now, I'd keep it.

[sffc]

It might be nice to remove FromStr for PluralOperands, since we are developing a more thoroughly tested code path for that feature in #270, but I think it's harmless to keep From<int-like>.

[filmil]

Thanks for explaining. Another question: is there a reason that FixedDecimal does not have a way to extract the integer part and the fractional part, but can rather extract only digit-wise?

[sffc]

This ticket is all about adding a way to convert the FixedDecimal to PluralOperands, which implies separating the integer and fraction part. FixedDecimal is a super simple representation that maps from magnitudes to digits. See #190 (comment):

To implement the conversion, you'd use digit_at(x) for x from 0 to the most significant digit to construct i; digit_at(x) for x from the least significant digit to -1 to construct f; and so forth.

[filmil]

This ticket is all about adding a way to convert the FixedDecimal to PluralOperands, which implies separating the integer and fraction part. FixedDecimal is a super simple representation that maps from magnitudes to digits. See #190 (comment):

Understood, but that is not quite the answer to my question. I was interested as to why FixedDecimal doesn't have functions, say, named integer_part and fractional_part. Is there something specific about the intended use of FixedDecimal that makes this undesirable?

[sffc]

I was interested as to why FixedDecimal doesn't have functions, say, named integer_part and fractional_part. Is there something specific about the intended use of FixedDecimal that makes this undesirable?

What type do you envision that those functions would return?

• Another FixedDecimal? What scale would you set on the FixedDecimal? Why would you not just use the first FixedDecimal?
• An integer? What if the number is too big to fit in an integer?

[filmil]

• Another FixedDecimal? What scale would you set on the FixedDecimal? Why would you not just use the first FixedDecimal?

I would use a compatible scale. It is possible to set the internal representation of a FixedDecimal such that the fractional part is zero. Move other internal representation bits to match.

I wanted to use this modified FixedDecimal because I'd be able to use the internal structures of FixedDecimal to compute the integer and fractional parts, faster than relying on digit_at to iterate over specific digits.

• An integer? What if the number is too big to fit in an integer?

We could signal an overflow. That said, for purposes of plural rules matching, most of the time only significant digits matter, and the exponents matter less so. It would be a different story for spellout, for example.

Either way, overflow is a realistic possibility for FixedDecimal even today, given that digits are stored in an 8-byte array.

[filmil]

I would use a compatible scale. It is possible to set the internal representation of a FixedDecimal such that the fractional part is zero. Move other internal representation bits to match.

That said, let me check in a benchmark first, then I can do an experiment to figure out if this even makes sense.

Top level info that I'd appreciate is whether FixedDecimal is required to have a specific API, or if you'd be supportive of adding more methods to it.

[sffc]

I wanted to use this modified FixedDecimal because I'd be able to use the internal structures of FixedDecimal to compute the integer and fractional parts, faster than relying on digit_at to iterate over specific digits.

digit_at is intended to be very fast. It is the operation that FixedDecimal is designed to be very good at doing. I think the function is like 2 or 3 lines of code. If we were to write some other helper function, it would use digit_at under the hood.

Top level info that I'd appreciate is whether FixedDecimal is required to have a specific API, or if you'd be supportive of adding more methods to it.

As usual, new APIs should solve problems. I don't see what problem integer_part and fractional_part are solving, other than being more convenient for the conversion to PluralOperands. I want to push back on efforts to make FixedDecimal more than what it is: a fast, low-level representation of decimal digits at a range of magnitudes (powers of 10).

[filmil]

than being more convenient for the conversion to PluralOperands. I want to push back on efforts to make FixedDecimal more than what it is: a fast, low-level representation of decimal digits at a range of magnitudes (powers of 10).

If you don't want to widen the API on FixedDecimal, that is OK and that answers my question.

[sffc]

There's really only one algorithm you need, which is:

int result = 0;
for (i in high..=low) {
    result = result * 10 + fixed_decimal.digit_at(i);
}
return result;

My mental model has been that this pattern is short and simple enough to put at the call site. It doesn't need its own method in FixedDecimal.

[zbraniecki]

Fixed by #278.

[sffc]

I'm having trouble linking #278 to this issue. Can someone else try?

[zbraniecki]

I also cannot