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

Is it planned to implement cross decimal numeric ops? #10

Open
MathisWellmann opened this issue Oct 16, 2024 · 4 comments
Open

Is it planned to implement cross decimal numeric ops? #10

MathisWellmann opened this issue Oct 16, 2024 · 4 comments

Comments

@MathisWellmann
Copy link
Contributor

MathisWellmann commented Oct 16, 2024

In principle something like:

impl<I, const D: u8, const D_RHS: u8> std::ops::Mul<Decimal<I, D_RHS>> for Decimal<I, D>{}

which would allow two decimals of differing constant decimal range to nicely compose together.
This does not compile, so we'd need another mechanism such as a new trait which does the conversion and numeric operation explicitly, thus avoiding confusion as well.
Obviously the operations would apply the proper scaling to ensure correctness.

The usecase I'm considering this for is in lfest-rs,
e.g for multiplying a currency denoted in BaseCurrency(Decimal<I, D>) by a fraction denoted in BasisPoints(Decimal<I, 4>).
But more generally this could be useful for computing a percentage (Decimal<u32, 2>) of a value with a different constant decimal point.

Is this is something you would consider accepting into the code?
I'd be happy to create a pull request for it.

@OliverNChalk
Copy link
Owner

Hi Mathis, this is indeed something I have considered but skipped over as my original use case was a uniform scaling across my trading system. However, unfortunately crypto exchanges insist on using very strange step sizes etc so that experiment failed. Hence I've now needed to add https://github.com/OliverNChalk/const-decimal/tree/feat/dyn-decimal to give me a wider range of support.

I think https://crates.io/crates/fixed implements a similar mechanism to what you're talking about but that is unfortunately in base2 so not useful for financial applications.

I would definitely accept a PR adding this. Let me first merge the dyn-decimal branch as it converts the repo into a workspace. Otherwise if you start working you'll get some merge conflicts if I do merge dyn-decimal.

@MathisWellmann
Copy link
Contributor Author

Sounds good to me. I'll experiment with my idea for a bit first and will let you know how it turns out.

@MathisWellmann
Copy link
Contributor Author

I played around with some ideas but ultimately decided to just use a single const D: u8 for all relevant types in lfest-rs, like BaseCurrency, QuoteCurrency, Fee etc, because the overhead of converting from one decimal scale to another would not be worth it in that case.
So I won't be further pursuing this in the future, but it can remain an open issue in case someone else picks it up.

P.S.:
After switching from the fpdec-rs decimal crate to const-decimal, the performance increased by 5-50x depending on the datatype used and the synthetic case being considered. Real world performance for backtesting has increased by 6x using i64 which is a big win for this crate given that fpdec was the previous champion in terms of performance as far as I know. The new types are much smaller as well, 8 bytes vs 32 bytes of fpdec::Decimal.
I got some benchmarks comparing the two here: https://github.com/MathisWellmann/lfest-rs/blob/f5df713a63a5988abd909239d578746334fc068c/benches/decimal_comparison.rs

Are you interested in having a benchmark comparing const-decimal with fpdec or other crates in this repo?
I got a PR essentially ready to go for such a case and this could be a nice selling point to showcase vs other decimal crates.

@OliverNChalk
Copy link
Owner

Would happily accept a PR showing performance compared to another crate

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

No branches or pull requests

2 participants