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

Improve performance and API with Decimals for very big and very small values #26

Closed
dpalic opened this issue Sep 21, 2017 · 7 comments
Closed
Labels
enhancement Issue describing or discussing an enhancement for this library question
Milestone

Comments

@dpalic
Copy link
Collaborator

dpalic commented Sep 21, 2017

Right now we have a custom implemented Decimal class. This class solves issues like:

  • MathContext
  • decimal precision

Sadly it still leaves out some other issues:

  • performance (dpalic I would really like to have some use cases in unit tests to have a better decision if any performance tuning is really needed with nowadays CPUs)
  • handling of international currencies is right now missing (e.g. you trade in EUR but you work on US-market, this is our current API not aware of)
  • using standard solutions (e.g. Joda money which very probably will be replaced with JSR-354 Money and Currency and will be probably integrated in JDK10)
  • plain BigDecimal

Personally I prefer using standards. Previously I used Joda Time, and the integration into the JDK made many things much easier. So I assume also that the integration of Money and Currency JSR will also make many things pretty simple. Here a short intro http://www.baeldung.com/java-money-and-currency

Also JSR-354 has already considerations about the tradeoff of performance and precision. E.g. the user of our API could himself decide which precision he/she wants to use by using either Money (BigDecimal based) or FastMoney (long based) implementation.
Since it will make its way into the JDK it will soon be obsolete as a additional dependency right it is now as the reference implementation.

Good readings for the decision:
mdeverdelhan/ta4j-origins#19, mdeverdelhan/ta4j-origins#28, mdeverdelhan/ta4j-origins#4 (comment)

Also related to this are: #24 and #21

The rework for this Decimal issue will take some time, therefore I would like to move it to version 0.11
What do you think about the proposed Java Money and Currency and about the move to release 0.11?

@dpalic dpalic added this to the 0.11 milestone Sep 21, 2017
@dpalic dpalic added enhancement Issue describing or discussing an enhancement for this library question labels Sep 21, 2017
@martinklepsch
Copy link
Contributor

martinklepsch commented Sep 21, 2017

I don't think introducing the concept of currencies via Joda-Money is a good idea currently.

Using standards is good and I was excited when I saw that ta4j used java.time. However I think that there is no reason to add currencies to ta4j. I'm sure it helps when you trade in the kind of situations you described but I would consider this more of a trade-management issue than something related to plain technical analysis. Not trying to say it's not useful just not sure it's strictly required in "core". Perhaps it can be explored as a separate library that is designed to be used with ta4j.

Even if we decide to add Joda-Money we will still need some regular Decimal class for trade volume and indicators returning regular numbers (e.g. any oscillating indicator).

In mdeverdelhan/ta4j-origins#28 Decimal4j was discussed as a Decimal alternative. As far as I understand the discussion the issue back then was that the maximum precision was Decimal10f. Looking at Decimal4j now it offers precision up to Decimal18f. Maybe this would be worth looking into again.

Some quotes by @mdeverdelhan from the issues you listed:

Most of indicator calculations don't manipulate money amounts. See SMA: you can compute your moving average on whatever indicator you want (trade count for instance) and not only on the close price or another "monetary" indicator.

The problem with money libraries is that they offer to manipulate "currencied" value. I want ta4j to be currency-agnostic. Also most of the indicator we are providing don't return monetary values.

@team172011
Copy link
Member

team172011 commented Sep 21, 2017

Why do you refer to #21 ?

@jonathanj
Copy link
Contributor

The @mdeverdelhan quotes are very relevant, after thinking about it I think there are several reasons I would be against the idea of adding currency:

  1. Currency precision (0 for JPY, 8 for BTC, etc.) is largely irrelevant for an indicator, in fact you probably want as much precision as you can reasonably get for indicators so that derived indicators can be most effective. Once a value comes out of an indicator it's up to the user to interpret that value, if it's monetary then at that point the user can feed it into a suitable type.
  2. Introducing a convertible unit, such as currency, requires additional care to avoid confusing behaviour (or avoid introducing bugs) where you need an arbitrary-precision decimal unit to interact with currency.
  3. Another type of number means more work, work that doesn't appear to have a concrete use-case.

@martinklepsch
Copy link
Contributor

martinklepsch commented Sep 21, 2017

Might be worth splitting this issue into

  • More performant Decimal implementation and
  • Introduction of currency / money abstraction

@team172011
Copy link
Member

@martinklepsch good idea, I think a more performant Decimal implementation with standard types is straight forward, too. But I am also concerned about introduction of currencies.

@jonathanj
Copy link
Contributor

jonathanj commented Sep 21, 2017

@mdeverdelhan pointed out that Decimal4j's implementation makes some tradeoffs for performance. Specifically because the underlying value is represented as a single long, you can either have precision to the left or to the right of the decimal point but not both.

From http://decimal4j.org/javadoc/1.0.3/org/decimal4j/api/Decimal.html:

Operations with Decimal values can lead to overflows in marked contrast to the BigDecimal. This is a direct consequence of the construction of a Decimal value on the basis of a long value. Unless otherwise indicated operations silently truncate overflows by default. This choice has been made for performance reasons and because Java programmers are already familiar with this behavior from operations with primitive integer types.

This seems undesirable for a library like ta4j, especially considering that this would be a premature optimisation (considering nobody has performance issues caused by the use of BigDecimal.)

@team172011
Copy link
Member

discussion moved to #113 #88 and #75

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issue describing or discussing an enhancement for this library question
Projects
None yet
Development

No branches or pull requests

4 participants