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

Fixed Point recipe #196

Merged
merged 44 commits into from
Mar 30, 2020
Merged

Fixed Point recipe #196

merged 44 commits into from
Mar 30, 2020

Conversation

JoshOrndorff
Copy link
Owner

@JoshOrndorff JoshOrndorff commented Mar 25, 2020

Teach users why we don't use floats, and some options for working around them.

Idea: Continuous and discrete compounding interest. Discrete compounding is paid out every ten block in on_finalize and uses Substrate's simple fixed point stuff (specifically Percent). Continuously compounding interest is calculated and paid out when deposits or withdraws are made and use the more advanced exponential features of https://github.com/encointer/substrate-fixed

@JoshOrndorff
Copy link
Owner Author

JoshOrndorff commented Mar 25, 2020

@brenzi Can you help me understand a bit about substrate-fixed? I'm trying to write a recipe about using fixed point math in Substrate, and I saw some postings about the cool transcendental functions in substrate-fixed. Commit 4199e2c has my attempts to use it, but I'm having some trouble. Here are some concrete questions.

  1. Why can I use the transcendental functions when I pull substrate-fixed from github, but not crates.io? (EDIT: I figured this one out. The docs that say to use crates.io are leftover from before you forked. Pulling the fixed crate from there is a totally different crate.)
  2. Why can I multiply a u32F32 by an "integer" but not a u32?
let a = U32F32::from_num(1);

// This works
let b = a * 2;

// This doesn't
let c = a * 2u32;
  1. How can I actually use this in practice? If I pull a number out of runtime storage it wil always have some type (like u32)

  2. (A little less concrete) - I'm ultimately trying to do a compounding interest calculation in my value_of_continuous_account function. I should return something like this, but I'm really getting my butt kicked by the type system.

principle * exp( interest_rate() * elapsed_time )

@coriolinus your help is also welcome. I know you recommended substrate-fixed. Not sure whether you've used it though.

@brenzi
Copy link
Contributor

brenzi commented Mar 26, 2020

@JoshOrndorff

  1. yes, fixed on crates.io is the upstream crate which doesn't implement parity_scale_codec and transcendental fn.
  2. fixed has limited traits to inteact with trivial types. Basically from_num, into_bits, lossy_into are your helpers.
  3. I'm using the crate for calculations of distances of geolocations. This can serve you as an example. Basically, you don't go back to u32. You just use the SCALE encoder to store fixed values and above helpers to move back and forth with u32 and alike
  4. let me know if the example above doesn't help you with this example.

@JoshOrndorff JoshOrndorff marked this pull request as ready for review March 27, 2020 13:34
@JoshOrndorff
Copy link
Owner Author

Also requesting review from @brenzi

@JoshOrndorff JoshOrndorff changed the title [WIP] Fixed Point recipe Fixed Point recipe Mar 27, 2020
pallets/fixed-point/src/lib.rs Show resolved Hide resolved
text/3-entrees/fixed-point.md Outdated Show resolved Hide resolved
text/3-entrees/fixed-point.md Show resolved Hide resolved
text/3-entrees/fixed-point.md Outdated Show resolved Hide resolved
text/3-entrees/fixed-point.md Show resolved Hide resolved
text/3-entrees/fixed-point.md Outdated Show resolved Hide resolved
text/3-entrees/fixed-point.md Show resolved Hide resolved
Copy link
Contributor

@coriolinus coriolinus left a comment

Choose a reason for hiding this comment

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

Looks great! I've found a few typos and quibbles, but in general they should all be very straightforward fixes.

That said, I have not performed an in-depth code review; I take it as given that the example pallets do what they say they will.

text/3-entrees/fixed-point.md Outdated Show resolved Hide resolved
pallets/fixed-point/src/lib.rs Outdated Show resolved Hide resolved
text/3-entrees/fixed-point.md Outdated Show resolved Hide resolved
text/3-entrees/fixed-point.md Outdated Show resolved Hide resolved
text/3-entrees/fixed-point.md Outdated Show resolved Hide resolved
text/3-entrees/fixed-point.md Outdated Show resolved Hide resolved
text/3-entrees/storage-api/enumerated.md Outdated Show resolved Hide resolved
JoshOrndorff and others added 6 commits March 27, 2020 11:01
Co-Authored-By: Peter Goodspeed-Niklaus <[email protected]>
Co-Authored-By: Peter Goodspeed-Niklaus <[email protected]>
Co-Authored-By: Peter Goodspeed-Niklaus <[email protected]>
Co-Authored-By: Peter Goodspeed-Niklaus <[email protected]>
Co-Authored-By: Peter Goodspeed-Niklaus <[email protected]>
@coriolinus
Copy link
Contributor

a88f091 just undid a bunch of the fixes in fixed-point.md ☹️

pallets/compounding-interest/Cargo.toml Outdated Show resolved Hide resolved
Copy link
Contributor

@joepetrowski joepetrowski left a comment

Choose a reason for hiding this comment

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

A few typos/nits, but LGTM. I'm a more comma-heavy writer and would use more commas in the text, especially before "which" clauses, but it's fine if you like it.

text/3-entrees/fixed-point.md Show resolved Hide resolved
text/3-entrees/fixed-point.md Outdated Show resolved Hide resolved
text/3-entrees/fixed-point.md Outdated Show resolved Hide resolved
text/3-entrees/fixed-point.md Outdated Show resolved Hide resolved
text/3-entrees/fixed-point.md Outdated Show resolved Hide resolved
text/3-entrees/fixed-point.md Outdated Show resolved Hide resolved
text/3-entrees/fixed-point.md Outdated Show resolved Hide resolved
text/3-entrees/fixed-point.md Outdated Show resolved Hide resolved
pallets/compounding-interest/src/lib.rs Outdated Show resolved Hide resolved
text/3-entrees/fixed-point.md Outdated Show resolved Hide resolved
text/3-entrees/fixed-point.md Outdated Show resolved Hide resolved
@JoshOrndorff JoshOrndorff merged commit 442c8ba into master Mar 30, 2020
@JoshOrndorff JoshOrndorff deleted the joshy-fixed-point branch March 30, 2020 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants