Ratios and rounding modes

[Chris-Hibbert]

The current Ratio library hides the fact that every multiplyBy and divideBy ends by making an Amount with a bigInt value by doing a divide. The current implementation always uses floorDivide rather than giving the caller control. We're in agreement that the caller should make an explicit choice, but haven't found operation names and parameter decisions that we all like. This issue is a place to be clear about potential solutions so we can settle on a least bad compromise, or find something that we mostly all like.

There are two operations amount * ratio and amount / ratio and two choices of rounding mode: round up and round down. We're pretty much agreed that using a parameter to choose either the rounding mode or the operation would be a mistake. That leaves naming the operation.

There are some arguments for scale or possibly apply as the operation name rather than multiple/divide.

1. ceilMultiplyBy, floorMultiplyBy, ceilDivideBy, floorDivideBy
2. ceilMultiplyBy, floorMultiplyBy
3. ceilScale, floorScale
4. ceilApply, floorApply
5. applyCeil, applyFloor
6. applyUp, applyDown

One alternative to having two operation names is to perform divisions by explicitly inverting the ratio. Current practice heavily leans toward multiplyBy; it seems relatively straightforward to arrange that the ratios we build are designed for use with multiplyBy. (I only see two non-test uses of divideBy, while there are ~20 of multiplyBy.)

Dean suggested:

A ratio naming idea I just got from looking at how I've named related variable: call it "rate" rather than ratio. That's a more economic term for the abstraction that both justifies and signals that it's not pure math. Then it's easier to have different operation names. rate.apply is actually coherent, though too vague.

He also suggested that it's possible that the ratio/rate could have the choice of floorDivide vs. ceilDivide baked in.

[Chris-Hibbert]

As the above maybe hints at, I'm currently leaning toward a single operation (and use of invertRatio), since we seem to be able to make the vast majority of operations be multiplies. I prefer scale over apply since it more clearly evokes the ratio nature of the operation. I'd prefer multiply if we didn't have to add the direction to the name.

I don't think up and down imply rounding as well as floor and ceil do.

[dckc]

I like the idea of

const { quotient: subtotal, remainder: fee } = AmountMath.split(bid, 0.97)

where one API call is used for both parts and ensures conservation of amounts.

Perhaps an optional round arg would default to ceil but allow floor.

ack: I think @dtribble suggested this.

[Chris-Hibbert]

const { quotient: subtotal, remainder: fee } = AmountMath.split(bid, 0.97)

Not all our ratios are less than one, so thinking of it as splitting the amount doesn't always make sense. When it does, I like the idea of giving the product and the remainder.

(quotient makes it sound like a divide rather than a multiply. There's an internal divide, but that's because we use 97/100 rather than 0.97). The operation we're most commonly performing is amount * ratio.

We could revisit the idea of merging this into AmountMath, but the current state is that Ratios are separate from Amounts, though the do overlap with that functionality a lot. The vast majority of uses of AmountMath never get near a ratio.

[katelynsills]

My vote is for ceilMultiplyBy, floorMultiplyBy, ceilDivideBy, floorDivideBy. It's simple, very clear, and is easy to use in our current use cases. I think clarity should trump aesthetic concerns, if those are opposed.

[dtribble]

I like the idea of

const { quotient: subtotal, remainder: fee } = AmountMath.split(bid, 0.97)

ack: I think @dtribble suggested this.

Yes. The basic idea is that the difference between "floor" and "ceil" is instead "choose which party you explicitly give the remainder to".

Edit: Actually the code sample entangled two separate suggestions:

• have a split option for fees
• have the rounding error be a separate result

I like 1, though as Chris notes, what you want for ratios<1 is usually different than rations > 1. 2 probably is not worth it, since generally we are just talking about whether the least significant bit is 1 or 0 :).

[Chris-Hibbert]

@erights, @dtribble Please express a position among options 1-6, or describe an alternative in enough detail that it could be considered with the others. @katelynsills expressed a preference for #1. My preference is for #3 or #2, though I'd prefer #1 to not making a decision.

IMHO, @dckc's suggestion addresses a subset of cases, so isn't really in contention as a general solution.

[erights]

I like #3 much better than the others. I had to ask @Chris-Hibbert to explain "apply" to me as I had no idea where that came from.

I also like "scale" better than "rate"

[dtribble]

I prefer #3 of the action options. I don't like starting operation names that don't start with the operation; e.g. ceilScale and floorScale. Is scaleCeil and scaleFloor better?

If we are using the verb scale, we can't call the noun scale. FWIW, for my usages, rate works well *(conversionRate, liquidationRate, etc.). For those usages, scale as a noun would be very strange.

[erights]

Scaling by a rate sounds good to me. So we'd still keep "scale" as the verb.

I think I like scaleCeil and scaleFloor better for the reason you suggest. My discomfort is that English has "adjective verb" as a verb phrase, but not "verb adjective". If others think it grates on their ears on those grounds, I'd stick with ceilScale and floorScale.

Everyone: Does scaleCeil and scaleFloor sound too weird because it is "verb adjective"?

[Chris-Hibbert]

I don't have a problem with scaleCeil and scaleFloor.

[dtribble]

BTW the one minor caveat for having the verb first is that tab completion will require you to choose each time whether ceiling or floor.

[katelynsills]

I don't think we have consensus yet, since I voted for number 1. There are significant downsides to number 3:

1. We tried the word scale already for this operation in production, and found it lacking because it was not clear. multiplyBy was a large improvement in clarity, which I experienced as a user of this.
2. Users want to be able to both multiply by and divide by ratios. If we only supply the multiply method, the user will have to inverse and then multiply. Essentially they will have to reimplement a divide function for themselves. It's a better user experience for us to provide that.

I think we need to prioritize clarity and ease of use, and 3 doesn't do that.

[dtribble]

I dislike using multiply that way, but I'm coming around to the perspective that it's nonetheless clearer. I recall the confusions around scale.

We do however have 93 multiplyBy vs 15 divideBy. Is that enough of a difference to migrate completely to mutiply? The Without tests: 38/7, and 11/5 in dapp-treasury.

[katelynsills]

We do however have 93 multiplyBy vs 15 divideBy. Is that enough of a difference to migrate completely to mutiply? The Without tests: 38/7, and 11/5 in dapp-treasury.

Trying to read this - does this mean there are 38 multiplyBy usages, and 7 divideBy usages outside of tests?

Is that enough of a difference to migrate completely to mutiply?

Can you expand on this question? Not sure what you mean.

[Chris-Hibbert]

I count 20 multiplyBy) outside of tests and 2 divideBy. (excluding comments, imports, declarations, etc.) Both of the divideBy calls are with liquidatioMargin. (It is also used in multiplyBy, so it's not simply a matter that the ratio should have been inverted.)

The fact that there are so few cases of divideBy is somewhat convincing to me that we could get by with just one operation and require invert. But @katelynsills is right that legibility should be paramount.

[Chris-Hibbert]

Dean suggested that verbs should go first, which would give us

7. multiplyByFloor, multiplyByCeil, divideByFloor, divideByCeil
8. scaleFloor, scaleCeil

There are still unretracted arguments above for requiring invert in order to divide, which gives

9. multiplyByFloor, multiplyByCeil, invert

My current feeling is that we aren't saving anyone any complication by going with 7. Choice 9 has fewer operations, but not fewer concepts. The maintenance and doc burden for us to support explicit divide operations is negligible.

[katelynsills]

Option 7 gets my vote because scale was tried as a name for this already and was found to be unclear, and each user should not have to reinvent ratio division.

[Chris-Hibbert]

My current feeling is that we aren't saving anyone any complication by going with 7. Choice 9 has fewer operations, but not fewer concepts. The maintenance and doc burden for us to support explicit divide operations is negligible.

This gets what I intended to write exactly backward.

We don't save anyone any trouble by only having multiply operations and not divide. Leaving out divideBy results in fewer operations, but not fewer concepts. The maintenance and doc burden for us to support explicit divide operations is negligible.

(Edited to add) I should say this explicitly: I'm in favor of 7.

[erights]

I prefer option 1 to 7, even though I generally prefer verb first. But consider the names of the option 7 verbs. For example
multiplyByFloor
suggests, almost demands, the misreading that we are multiplying something by the floor of something. Whereas
floorMultiply (or floorMultiplyBy)
can be read two ways, both of which are correct:

• adjective multiply (or multiplyBy), where floor says what flavor of multiply.
• functional composition, where it is a conceptual contraction of floor(multiply(...)) (or floor(multiplyBy(...))

As a variant of option 7, multiplyFloor would be less confusing than multiplyByFloor.

[Chris-Hibbert]

10. multiplyFloor, multiplyCeil, divideFloor, divideCeil

[erights]

Separate but related question. Why the "By"? For an infix operator this is sometimes appropriate, as in the recent incrementBy. But for a prefix operation it seems weird.

[erights]

Have we already enumerated

11. floorMultiply, ....

[Chris-Hibbert]

We needed By in multiplyBy and divideBy to hint that the second argument was the ratio. The order doesn't change the meaning with multiply(amount, ratio), but with divideBy(amount, ratio), it's crucial.

[erights]

Rounding aside, is divide(amount, ratio) mathematically accurate? What extra does the "By" convey?

[Chris-Hibbert]

It was the fact that the ratio had to be the second argument to the divide operation that drove us away from making ratios be objects with methods.

One could say "divide x by y" or "divide y into x", so 'by' does add something.

[erights]

In light of Chris' defense of "By", I favor option 1. If we need the "By", that kills both 10 and 11. With the "By", the confusion of what's modifying what kills 7.

[erights]

If we agree

• We need all four operators
• We need the "By"
• multiplyByFloor and such is too confusing

then, are we left with any options other than 1?

[Chris-Hibbert]

I think we've also given up on apply and scale, as well as up and down, so I don't think there are any other live choices.

[katelynsills]

I'm fine with Option 1

[erights]

Any remaining objections to 1? If so, are there any surviving viable alternatives to 1?

[dtribble]

No objections.

[Chris-Hibbert]

I will add this to my tasks. Thanks for the discussion!

[Chris-Hibbert]

I'm going to need to think about how to represent ratios to multiply by non-amounts. In governance, a quorum is a ratio applied to numbers of voters, which don't have a brand.

[dtribble]

It seems to me that it has a brand. The quorum of a committee is unrelated to the quorum of a liq token vote. That brand mechanism allows describing that!