Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

osmomath: AddMut and QuoMut #3779

Merged
merged 13 commits into from
Dec 22, 2022
Merged

osmomath: AddMut and QuoMut #3779

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
28ac3a7
mut add
pysel Dec 17, 2022
e0e2601
test add mut
pysel Dec 17, 2022
4978cbe
quo mut
pysel Dec 17, 2022
2870114
test quo mut/ remove want from test struct
pysel Dec 17, 2022
2faea41
refactor exp
pysel Dec 17, 2022
0993272
change mutatives code
pysel Dec 17, 2022
5fec9f4
change
pysel Dec 17, 2022
0a20620
not allocaing
pysel Dec 18, 2022
0b22fd0
exp change to quomut
pysel Dec 18, 2022
8be5795
remove file
pysel Dec 20, 2022
f280099
refactor quo
pysel Dec 20, 2022
8b11ef6
refactor ad
pysel Dec 20, 2022
336e7b6
refactor tests
pysel Dec 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions osmomath/decimal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,17 @@ func (d BigDec) Add(d2 BigDec) BigDec {
return BigDec{res}
}

// mutative addition
func (d BigDec) AddMut(d2 BigDec) BigDec {
pysel marked this conversation as resolved.
Show resolved Hide resolved
d.i.Add(d.i, d2.i)

if d.i.BitLen() > maxDecBitLen {
panic("Int overflow")
}

return d
}

// subtraction
func (d BigDec) Sub(d2 BigDec) BigDec {
res := new(big.Int).Sub(d.i, d2.i)
Expand Down Expand Up @@ -332,6 +343,20 @@ func (d BigDec) Quo(d2 BigDec) BigDec {
return BigDec{chopped}
}

// mutative quotient
func (d BigDec) QuoMut(d2 BigDec) BigDec {
pysel marked this conversation as resolved.
Show resolved Hide resolved
// multiply precision twice
mul := new(big.Int).Mul(d.i, precisionReuse)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not allocate new int. One of the goals of the mutative versions is to gain speed up by avoiding reallocations and copying.

Please see:
https://github.com/osmosis-labs/cosmos-sdk/blob/48fe175f983925ff6202d492a363d2b2e3a62497/types/decimal.go#L343-L352

mul.Mul(mul, precisionReuse)

d.i.Quo(mul, d2.i)
d.i = chopPrecisionAndRound(d.i)

if d.i.BitLen() > maxDecBitLen {
panic("Int overflow")
}
return d
}
func (d BigDec) QuoRaw(d2 int64) BigDec {
// multiply precision, so we can chop it later
mul := new(big.Int).Mul(d.i, precisionReuse)
Expand Down
54 changes: 54 additions & 0 deletions osmomath/decimal_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,60 @@ func (s *decimalTestSuite) assertMutResult(expectedResult, startValue, mutativeR
s.Require().Equal(nonMutativeStartValue, startValue)
}

func TestAddMut(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since Add is supposed to call AddMut, can you please do a short test like this instead of duplicating test cases?

tests := []struct {
testing osmomath.BigDec
adding osmomath.BigDec
}{
{osmomath.NewBigDec(0), osmomath.NewBigDec(0)},
{osmomath.NewBigDec(1), osmomath.NewBigDec(1)},
{osmomath.NewBigDec(10), osmomath.NewBigDec(1)},
{osmomath.NewBigDec(12340), osmomath.NewBigDec(10)},
{osmomath.NewDecWithPrec(12340, 4), osmomath.NewDecWithPrec(12340, 4)},
{osmomath.NewDecWithPrec(12340, 5), osmomath.NewDecWithPrec(12, 5)},
{osmomath.NewDecWithPrec(12340, 8), osmomath.NewDecWithPrec(1009009009009009009, 8)},
{osmomath.NewDecWithPrec(1009009009009009009, 17), osmomath.NewDecWithPrec(100, 17)},
}
for _, tc := range tests {
want := tc.testing.Add(tc.adding)
tc.testing.AddMut(tc.adding)

require.Equal(t, want, tc.testing)
}
}

func TestQuoMut(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same for this test case

tests := []struct {
testing osmomath.BigDec
quo osmomath.BigDec

expectingErr bool
}{
{osmomath.NewBigDec(0), osmomath.NewBigDec(0), true},
{osmomath.NewBigDec(1), osmomath.NewBigDec(1), false},
{osmomath.NewBigDec(10), osmomath.NewBigDec(1), false},
{osmomath.NewBigDec(12340), osmomath.NewBigDec(10), false},
{osmomath.NewDecWithPrec(12340, 4), osmomath.NewDecWithPrec(12340, 4), false},
{osmomath.NewDecWithPrec(12340, 5), osmomath.NewDecWithPrec(12, 5), false},
{osmomath.NewDecWithPrec(1009009009009009009, 8), osmomath.NewDecWithPrec(1000, 8), false},
{osmomath.NewDecWithPrec(500, 17), osmomath.NewDecWithPrec(100, 17), false},
}
for _, tc := range tests {
if !tc.expectingErr {
want := tc.testing.Quo(tc.quo)
fmt.Println(tc.testing, tc.quo)
tc.testing.QuoMut(tc.quo)

require.Equal(t, want, tc.testing)
} else {
save := tc.testing

require.Panics(t, func() { tc.testing.QuoMut(tc.quo) })
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a helper for this here:

osmosis/osmomath/export_test.go

Line 25 in 6206709

func ConditionalPanic(t *testing.T, expectPanic bool, sut func()) {

require.Equal(t, save, tc.testing)
}

}
}
func TestDecApproxEq(t *testing.T) {
// d1 = 0.55, d2 = 0.6, tol = 0.1
d1 := osmomath.NewDecWithPrec(55, 2)
Expand Down
4 changes: 2 additions & 2 deletions osmomath/exp2.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,8 @@ func exp2ChebyshevRationalApprox(x BigDec) BigDec {
for i := 1; i < len(numeratorCoefficients13Param); i++ {
x_exp_i.MulMut(x)

h_x = h_x.Add(numeratorCoefficients13Param[i].Mul(x_exp_i))
p_x = p_x.Add(denominatorCoefficients13Param[i].Mul(x_exp_i))
h_x.AddMut(numeratorCoefficients13Param[i].Mul(x_exp_i))
p_x.AddMut(denominatorCoefficients13Param[i].Mul(x_exp_i))
pysel marked this conversation as resolved.
Show resolved Hide resolved
}

return h_x.Quo(p_x)
Expand Down