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.

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

feat(osmomath): Exp2 function #3708

Merged
merged 20 commits into from
Dec 16, 2022
Merged

feat(osmomath): Exp2 function #3708

merged 20 commits into from
Dec 16, 2022

Conversation

p0mvn
Copy link
Member

@p0mvn p0mvn commented Dec 14, 2022

Closes: #XXX

What is the purpose of the change

This PR adds an Exp2 function that computes $$2^{decimal}$$

The following property is used:
$$2^{decimal} = 2^{integer} * 2^{fractional}$$

The integer exponent result is computed using the existing PowerInteger function.

The fractional exponent result is computed using the 13-parameter Chebyshev rational approximation. The numerator coefficients are rounded down and the denominator coefficients are rounded up at the precision end. However, this does not guarantee that the approximated result is always smaller than the actual results (as determined by WolframAlpha).

The Python approximation script is modified to utilize the production coefficients for estimating relative errors. Please see the results:
image

Additionally, ErrTolerance was refactored to use sdk.Dec for AdditiveTolerance for more accuracy in tests.

In a future PR, Power function with a decimal exponent can be added. This function will use exp2 by utilizing the following property:
x^y = exp2(y * log_2{x})

Testing and Verifying

Tests with links to WolframAlpha were added. The absolute and the relative errors are bounded.

Open Questions

  • Is it acceptable that we cannot guarantee rounding behavior?

Documentation and Release Note

  • Does this pull request introduce a new feature or user-facing behavior changes? no
  • Is a relevant changelog entry added to the Unreleased section in CHANGELOG.md? yes
  • How is the feature or change documented? godocs and approximation scripts

@p0mvn p0mvn changed the title feat(osmomath): exp2 function feat(osmomath): Exp2 function Dec 14, 2022
@github-actions github-actions bot added the C:x/gamm Changes, features and bugs related to the gamm module. label Dec 14, 2022
@p0mvn p0mvn force-pushed the roman/improved-pow6 branch from 0cc8250 to a548068 Compare December 14, 2022 02:03
@github-actions github-actions bot removed the C:x/gamm Changes, features and bugs related to the gamm module. label Dec 14, 2022
@p0mvn p0mvn added V:state/compatible/backport State machine compatible PR, should be backported A:backport/v13.x backport patches to v13.x branch labels Dec 14, 2022
@github-actions github-actions bot added the C:x/gamm Changes, features and bugs related to the gamm module. label Dec 14, 2022
osmomath/exp2.go Outdated Show resolved Hide resolved
osmomath/exp2.go Outdated Show resolved Hide resolved
osmomath/exp2.go Outdated Show resolved Hide resolved
osmomath/exp2_test.go Outdated Show resolved Hide resolved
osmomath/exp2_test.go Outdated Show resolved Hide resolved
@p0mvn p0mvn requested a review from ValarDragon December 14, 2022 05:29
@p0mvn p0mvn marked this pull request as ready for review December 14, 2022 05:29
@nicolaslara
Copy link
Contributor

Is it acceptable that we cannot guarantee rounding behavior?

I think as long as the error is within the an acceptable range (from the chart $[10^{-22}, 10^{-27}]$), then I think it doesn't matter if it's higher or lower than the actual results

@github-actions github-actions bot removed the C:x/gamm Changes, features and bugs related to the gamm module. label Dec 14, 2022
@p0mvn p0mvn force-pushed the roman/improved-pow6 branch from 1cd4928 to 79beaae Compare December 14, 2022 21:47
@p0mvn
Copy link
Member Author

p0mvn commented Dec 15, 2022

Is it acceptable that we cannot guarantee rounding behavior?

I think as long as the error is within the an acceptable range (from the chart [10−22,10−27]), then I think it doesn't matter if it's higher or lower than the actual results

There are some cases where the error is outside of the chart - 10^17. For example:

AdditiveTolerance: sdk.MustNewDecFromStr("0.00000000000000007"),

I have not been able to explain why that is the case.

One guess is that approximated results in Go are being compared against WolframAlpha. The chart is constructed from a Python library where both the approximations and the actual results are using the same framework. I'm wondering if different computational platforms introduce additional errors.

Please let me know your thoughts.

@p0mvn
Copy link
Member Author

p0mvn commented Dec 15, 2022

This is how the relative error plot looks like (very similar if not identical to the absolute):
Screenshot from 2022-12-15 14-53-00

From call, the concern was that relative and absolute plots should not look the same.

On a quick review, I do not see a bug in the way it is calculated. However, I'm going to investigate this better and get back with updates.


func TestExp2ChebyshevRationalApprox(t *testing.T) {
smallValue := osmomath.MustNewDecFromStr("0.00001")
smallerValue := osmomath.MustNewDecFromStr("0.00000000000000000001")
Copy link
Member

Choose a reason for hiding this comment

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

What is this smaller than & what is relation to minDecTolerance (sorry not sure if theres some important relation)

Copy link
Member Author

Choose a reason for hiding this comment

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

I used these to implement test cases closer to the boundaries.

The reason is that the errors tend to be large at the boundaries for non-Chebyshev approximations. This is just to make sure that, as expected, Chebyshev does not allow for larger errors close to boundaries. I will add a comment relaying this

osmomath/exp2.go Outdated Show resolved Hide resolved
@ValarDragon
Copy link
Member

Python and actual go code both LGTM, nice job! Still reviewing go tests

@ValarDragon
Copy link
Member

The plot difference LGTM

Were actually seeing what we'd expect to see between the absolute error vs relative error

The additive error on RHS side was larger than LHS. The relative error 'dampens' the RHS relative to the LHS, as exp only increasing on the range

osmomath/exp2_test.go Outdated Show resolved Hide resolved
Copy link
Member

@ValarDragon ValarDragon left a comment

Choose a reason for hiding this comment

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

Nice job!

osmomath/exp2.go Outdated Show resolved Hide resolved
@p0mvn
Copy link
Member Author

p0mvn commented Dec 16, 2022

The plot difference LGTM

Were actually seeing what we'd expect to see between the absolute error vs relative error

The additive error on RHS side was larger than LHS. The relative error 'dampens' the RHS relative to the LHS, as exp only increasing on the range

That makes sense. Thanks for confirming

@p0mvn
Copy link
Member Author

p0mvn commented Dec 16, 2022

All comments are now addressed. This should be ready for merge once CI passes.

@p0mvn p0mvn merged commit 89bf606 into main Dec 16, 2022
@p0mvn p0mvn deleted the roman/improved-pow6 branch December 16, 2022 23:53
mergify bot pushed a commit that referenced this pull request Dec 16, 2022
* feat(osmomath): exp2 function

* export exp2

* changelog

* refactor ErrTolerance to use Dec instead of Int for additive tolerance

* Update osmomath/exp2.go

* Update osmomath/exp2.go

* Update osmomath/exp2.go

* Update osmomath/exp2_test.go

* Update osmomath/exp2_test.go

* do bit shift instead of multiplication

* godoc about error bounds

* comment about bit shift equivalency

* merge conflict

* improve godoc

* typo

* remove TODOs - confirmed obsolete

* Runge's phenomenon comment

(cherry picked from commit 89bf606)

# Conflicts:
#	CHANGELOG.md
czarcas7ic pushed a commit that referenced this pull request Jan 4, 2023
* feat(osmomath): exp2 function

* export exp2

* changelog

* refactor ErrTolerance to use Dec instead of Int for additive tolerance

* Update osmomath/exp2.go

* Update osmomath/exp2.go

* Update osmomath/exp2.go

* Update osmomath/exp2_test.go

* Update osmomath/exp2_test.go

* do bit shift instead of multiplication

* godoc about error bounds

* comment about bit shift equivalency

* merge conflict

* improve godoc

* typo

* remove TODOs - confirmed obsolete

* Runge's phenomenon comment
czarcas7ic added a commit that referenced this pull request Jan 5, 2023
…14 (#3925)

* Upgrade IBC to v4.2.0 (#3838)

* initial changes to migrate to ibc v4

* added checksum to proposal

* begin and end block are now being called inside nextBlock

* added changelog

* linked pr on changelog

* remove local replace

* using error acks from osmoutils

* osmoutils tagged

* go sum

* added checksum

* feat(x/twap): modify cli to add geometric option (#3812)

* feat(x/twap): geometric twap code gen query boilerplate

* revert cli change

* query gen

* wire up API

* test

* fix

* fixes

* cli

* lint

* refactor via flag

* refactor

* refactor

* fixes

* lint

* add arithmetic twap alias

* Make wasm hooks importable (#3850)

* moved ibc-hooks to its own go.mod

* updated ibc hooks version

* go sum

* add ics23 patch into x/ibc-hooks

* Fix wasm import version conflict

* Update x/ibc-hooks to osmoutils v0.0.2

* Update versions

Co-authored-by: Dev Ojha <dojha@berkeley.edu>

* refactor(x/twap): handle spot price error case in the context of geometric twap (#3845)

* refactor(x/twap): handle spot price error case

* supporting test cases

* table-driven log tests

* test(x/twap): add randomized geometric twap test on a balancer pool (#3844)

* test(x/twap): add randomized test with a balancer pool

* comments

* multiplicative tolerance, fewer retries and larger initial supply range

* Basic geometric twap e2e test (#3835)

* feat(x/twap): geometric twap code gen query boilerplate

* revert cli change

* query gen

* wire up API

* test

* fix

* fixes

* add geometric queries

* create pool.json

* add test

* resolve conflict

* fix: swap uosmo in

* fix problem with wallet creation

* updates

* simplify and add comments

* Update tests/e2e/e2e_test.go

* Update tests/e2e/e2e_test.go

* Update tests/e2e/configurer/chain/queries.go

Co-authored-by: Adam Tucker <adam@osmosis.team>

* Update tests/e2e/configurer/chain/queries.go

Co-authored-by: Adam Tucker <adam@osmosis.team>

* Update tests/e2e/e2e_test.go

Co-authored-by: Adam Tucker <adam@osmosis.team>

* Update tests/e2e/e2e_test.go

Co-authored-by: Adam Tucker <adam@osmosis.team>

* Update tests/e2e/e2e_test.go

Co-authored-by: Adam Tucker <adam@osmosis.team>

* Update tests/e2e/e2e_test.go

Co-authored-by: Adam Tucker <adam@osmosis.team>

Co-authored-by: Roman <ackhtariev@gmail.com>
Co-authored-by: Roman <roman@osmosis.team>
Co-authored-by: Adam Tucker <adam@osmosis.team>

* feat(x/twap): whitelist GeometricTwap and GeometricTwapToNow (#3852)

* feat(x/twap): GeometricTwap and GeometricTwapToNow queries added to Stargate whitelist

* update docs

* fix(scripts): proto gen for osmoutils (#3854)

* fix(scripts): proto gen for osmoutils

* Update scripts/protocgen.sh

* fix(scripts): proto gen osmoutils path (#3859)

* added packet timeouts to wasm hooks (#3862)

* add negative to cli (#3888)

* Making osmoutils compile on chains that don't use our SDK fork (#3899)

* making osmoutils compile on chains that don't use osmosis' fork of the cosmos sdk

* updated imports for e2e tests

* go fumpt

* updated version everywhere

* added changelog entry

* remove deprecation from arithmetic & geometric twap to now query (#3917)

* Add types & boilerplate for the Downtime detector module (#3609)

Sub-component of #3603

Adds types for the thin module intended for downtime detection

- Add downtime detection module types

No tests added

  - Does this pull request introduce a new feature or user-facing behavior changes? somewhat
  - Is a relevant changelog entry added to the `Unreleased` section in `CHANGELOG.md`? yes
  - How is the feature or change documented? In its spec

* Add downtime detector module (#3688)

* WIP

* Switch to enum

* Remove params query

* Add query

* Add wiring, add import/export test

* Start begin block test

* Finish keeper tests

* Add CLI

* Wire downtime detector CLI + queries

* more module wiring

* add types test

* Fix state alteration test

* Fix superfluid test

* Add stargate whitelist support

* Apply code comment

* Rename folder

* Add requested godoc

* Update x/downtime-detector/genesis.go

Co-authored-by: Adam Tucker <adam@osmosis.team>

* Apply adam suggestion for having a `-`

* move genesis test to own file

Co-authored-by: Adam Tucker <adam@osmosis.team>

* Initial by hand fixes

* feat(osmomath): Exp2 function (#3708)

* feat(osmomath): exp2 function

* export exp2

* changelog

* refactor ErrTolerance to use Dec instead of Int for additive tolerance

* Update osmomath/exp2.go

* Update osmomath/exp2.go

* Update osmomath/exp2.go

* Update osmomath/exp2_test.go

* Update osmomath/exp2_test.go

* do bit shift instead of multiplication

* godoc about error bounds

* comment about bit shift equivalency

* merge conflict

* improve godoc

* typo

* remove TODOs - confirmed obsolete

* Runge's phenomenon comment

* [x/TWAP] Expose a geometric TWAP API  (#3529)

* refactored twap api.go for geometric TWAP

* added barebon docs

* romans feedback

* new

* fix

* nichola feedback

* final roman comments

* fix twap by hand

* change to gamm

* fix balancer test

* bump to v14 upgrade

* e2e fix

* add remaining diff from main to ibc-rate-limit

* update contracts test

* osmomath: `AddMut` and `QuoMut` (#3779)

* mut add

* test add mut

* quo  mut

* test quo mut/ remove want from test struct

* refactor exp

* change mutatives code

* change

* not allocaing

* exp change to quomut

* remove file

* refactor quo

* refactor ad

* refactor tests

* Modify CHANGELOG

* Whitelist EstimateSwapExactAmountOut (#3693)

* whitelist EstimateSwapExactAmountOut

* doc: changelog

* updated rate limit contract

* Fix rust checks (#3576)

* added cargo.lock

* added Cargo.lock as an artifact

* added new bytecode with lock file

Co-authored-by: Nicolas Lara <nicolaslara@gmail.com>
Co-authored-by: Roman <roman@osmosis.team>
Co-authored-by: Dev Ojha <dojha@berkeley.edu>
Co-authored-by: Hieu Vu <72878483+hieuvubk@users.noreply.github.com>
Co-authored-by: Roman <ackhtariev@gmail.com>
Co-authored-by: Supanat <supanat.ptk@gmail.com>
Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>
Co-authored-by: Sishir Giri <sis1001@berkeley.edu>
Co-authored-by: Ruslan Akhtariev <46343690+RusAkh@users.noreply.github.com>
Co-authored-by: mattverse <mattpark1028@gmail.com>
Co-authored-by: ByeongSu Hong <frostornge@gmail.com>
@github-actions github-actions bot mentioned this pull request Apr 1, 2024
@github-actions github-actions bot mentioned this pull request May 1, 2024
@github-actions github-actions bot mentioned this pull request May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A:backport/v13.x backport patches to v13.x branch F: geometric-twap V:state/compatible/backport State machine compatible PR, should be backported
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants