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): Power with decimal exponent #3731

Merged
merged 20 commits into from
Dec 21, 2022
Merged

Conversation

p0mvn
Copy link
Member

@p0mvn p0mvn commented Dec 14, 2022

Closes: #XXX

What is the purpose of the change

Implements Power and PowerMut with a decimal exponent. Tests added.

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? no
  • How is the feature or change documented? not applicable

Base automatically changed from roman/improved-pow6 to main December 16, 2022 23:53
Comment on lines 1034 to 1041
// PowerMut returns a result of raising the given big dec to
// a positive decimal power. Panics if the power is negative.
// Panics if the base is negative. Mutates the receiver.
// N.B.: support for negative power can be added when needed.
func (d BigDec) PowerMut(power BigDec) BigDec {
d.i.Set(d.Power(power).i)
return d
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Unsure if there is value in this

@p0mvn p0mvn added the V:state/compatible/no_backport State machine compatible PR, depends on prior breaks label Dec 17, 2022
@p0mvn p0mvn marked this pull request as ready for review December 17, 2022 22:24
osmomath/decimal.go Outdated Show resolved Hide resolved
Comment on lines 1050 to 1053
func (d BigDec) PowerMut(power BigDec) BigDec {
d.i.Set(d.Power(power).i)
return d
}
Copy link
Member

Choose a reason for hiding this comment

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

err this is backwards. We should write the mutative method here, and then have Power just make a copy then run PowerMut

Copy link
Member

Choose a reason for hiding this comment

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

Or just delete power mut for now

Unless the idea is to keep this as an API stub, to optimize later?

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'm happy to delete this for now.

Since we rely on Exp2 function for the underlying implementation, making a mutative PowerMut is not as trivial as making MulMut.

@p0mvn p0mvn changed the title feat(osmomath): Power and PowerMut with decimal exponent feat(osmomath): Power with decimal exponent Dec 20, 2022
@p0mvn
Copy link
Member Author

p0mvn commented Dec 20, 2022

Addressed comments by removing PowerMut and creating #3800 to revisit error bounds in the future

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
Co-authored-by: Dev Ojha <[email protected]>
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.

LGTM! Can later support negative powers if need arises

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F: geometric-twap V:state/compatible/no_backport State machine compatible PR, depends on prior breaks
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants