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

Zig Zag indicator implementation question #1122

Open
francescopeloi opened this issue Nov 4, 2023 · 3 comments
Open

Zig Zag indicator implementation question #1122

francescopeloi opened this issue Nov 4, 2023 · 3 comments
Labels

Comments

@francescopeloi
Copy link
Contributor

francescopeloi commented Nov 4, 2023

Hello, I am not yet an expert of indicators and neither ta4j, and trying to develop my first indicator: the Zig Zag indicator.

This indicator seems to me peculiar, since with new close price values coming in, it might change previous calculated indicator values. This is due to the logic this indicator works.

In a nutshell:

data = {1.0, 1.1, 1.4}
zigzag(0) = 1.0
zigzag(2) = 1.4

in this case there's a (zigzag) line from 1.0 to 1.4

data = {1.0, 1.1, 1.4, 2.0}
zigzag(0) = 1.0
zigzag(3) = 2.0

in this case, where a new value of 2.0 comes in, the (zigzag) line is drawn from 1.0 to 2.0, which implies that also previous values are different, as the line inclination is now different. So zigzag(2) is not going to be 1.4 anymore, like stated in the previous example.

Give the above, in a test like this:

BarSeries data = new MockBarSeries(numFunction, 1.0, 1.0, 1.3, 1.5, 1.6, 1.8 , 0.9, 0.8, 0.8, 0.7, 1.0, 1.1, 1.1, 1.2);

where I provide all the test data in advance, should the indicator know all values? Or it should answer knowing the values only "incrementally"?

In other words: zigzag(4) knows only data[] from data[0] to data[4] or it knows data from data[0] to data[data.length]?

Also do you know any already implemented indicator that works similarly so I can take inspiration?

I have other questions but will start with these and see if I get traction 😄

Thanks for your help.

@francescopeloi
Copy link
Contributor Author

francescopeloi commented Nov 6, 2023

I think I answered myself.

where I provide all the test data in advance, should the indicator know all values? Or it should answer knowing the values only "incrementally"?

the indicator should know all the values. The barSerie the indicator provides has a beginIndex and endIndex. Even if the caller is asking for e.g. zigzag(1), endIndex could be 10, which means that further points are provided and should be taken into account in the indicator calculations.

Also do you know any already implemented indicator that works similarly so I can take inspiration?

I've found DifferencePercentageIndicator that seems similar/useful. I don't get if the use of the instance variable lastNotification is legit, given that there was a plan to remove them all, but it seems that it has been left there after the refactoring to remove instance variables. Still have to figure out why.

I'll continue developing with these assumptions.

@TheCookieLab
Copy link
Member

@francescopeloi you are correct, the indicator should be aware of all the values. There actually used to be a ZigZagIndicator class in the project but it was removed at some point. I don't know the reason but I assume it's because it didn't work quite as expected. That said here is the original code which may provide some value for you (i.e. things to borrow, things to avoid, etc) during your implementation:


package org.ta4j.core.indicators;

import org.ta4j.core.Indicator;
import org.ta4j.core.num.NaN;
import org.ta4j.core.num.Num;

/**
 * The Zig Zag Indicator.
 * <p>
 * For more information, see
 * <a href="https://www.investopedia.com/terms/z/zig_zag_indicator.asp">Zig Zag
 * Indicator</a>.
 */
public class ZigZagIndicator extends CachedIndicator<Num> {

    /**
     * The threshold ratio (positive number).
     */
    private final Num thresholdRatio;

    /**
     * The indicator to provide values.
     * <p>
     * Can be {@link org.ta4j.core.indicators.helpers.ClosePriceIndicator} or
     * something similar.
     */
    private final Indicator<Num> indicator;

    private Num lastExtreme;

    /**
     * Constructs a ZigZagIndicator.
     *
     * @param indicator the indicator to provide values
     * @param thresholdRatio the threshold ratio, must be positive
     */
    public ZigZagIndicator(Indicator<Num> indicator, Num thresholdRatio) {
        super(indicator);
        this.indicator = indicator;
        validateThreshold(thresholdRatio);
        this.thresholdRatio = thresholdRatio;
    }

    /**
     * Validates the provided threshold.
     *
     * @param threshold the threshold to validate
     * @throws IllegalArgumentException if threshold is null or non-positive
     */
    private void validateThreshold(Num threshold) {
        if (threshold == null) {
            throw new IllegalArgumentException("Threshold ratio is mandatory");
        }
        if (threshold.isNegativeOrZero()) {
            throw new IllegalArgumentException("Threshold ratio value must be positive");
        }
    }

    /**
     * Calculates the Zig Zag indicator's value for the current index.
     *
     * @param index the bar index
     * @return the indicator's value if its ratio from the last extreme is equal
     * to or greater than the threshold, otherwise {@link NaN}
     */
    @Override
    protected Num calculate(int index) {
        if (index == 0) {
            lastExtreme = indicator.getValue(0);
            return lastExtreme;
        } else {
            if (lastExtreme.isZero()) {
                // Treat this case separately
                // because one cannot divide by zero
                return NaN.NaN;
            } else {
                Num indicatorValue = indicator.getValue(index);
                Num differenceRatio = indicatorValue.minus(lastExtreme).dividedBy(lastExtreme);

                if (differenceRatio.abs().isGreaterThanOrEqual(thresholdRatio)) {
                    lastExtreme = indicatorValue;
                    return lastExtreme;
                } else {
                    return NaN.NaN;
                }
            }
        }
    }
}



@francescopeloi
Copy link
Contributor Author

francescopeloi commented Nov 9, 2023

thanks for your answer @TheCookieLab . I found that indicator you posted, but couldn't make it work properly, so started from scratch. I think this indicator can be implemented easily in a naive way, the problem is that if you don't look close, you're not going to find some hard-to-find edge cases that if you don't solve, what you get is just wrong (some examples: 2 high zigzag points after each other, you need to re-draw the previous line; or another one: price is exactly the same for some time: is it a high swing point o a low swing point? and which of those?). I am in the middle of trying to solve them in my first implementation, than I have to step back and see the whole picture again and probably write it again, as the code now looks awful :)

Can you help me understand one thing: I've found DifferencePercentageIndicator that seems similar. I want to undestand how it works to get some inspiration. I don't get if the use of the instance variable lastNotification there is legit, given that there was a plan to remove them (#906), but it seems that it has been left there after the PR (#1060) to remove instance variables.

Do you know why that instance variable would be acceptable? I thought that all instance variables should be removed from all indicators, given that even if you are using one, you're breaking statelessness, which we should not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants