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

Chandelier Exit indicator #58

Merged
merged 4 commits into from May 27, 2019
Merged

Chandelier Exit indicator #58

merged 4 commits into from May 27, 2019

Conversation

rdbuf
Copy link

@rdbuf rdbuf commented Mar 30, 2019

Sources:

  • Alexander Elder. Come Into My Trading Room, 2002, pp. 180-181. ISBN: 9780471225348
  • J. Welles Wilder. New Concepts in Technical Trading Systems, 1978, pp. 21-23. ISBN: 9780894590276
  1. True Range:

ce-1

  1. Average True Range:

ce-2

  1. Chandelier Exit, ce_high:

ce-4

  1. Chandelier Exit on short positions, ce_low:

ce-5

indicators/ce.c Outdated Show resolved Hide resolved
indicators/ce.c Outdated Show resolved Hide resolved
indicators/ce.c Outdated Show resolved Hide resolved
@codeplea
Copy link
Member

This looks pretty good. Great job with the documentation!

Here's my suggestion:

Instead of using your MAX3 and TR macros, just include "truerange.h" and use the CALC_TRUERANGE macro. I just benchmarked it, and ce went from 59 mfps to 77 mfps with this change (using -O2). It also keeps it more consistent with the rest of the codebase.

You'll need 'TI_REAL truerange;` at the top, then the first loop would look like this:

    for (i = 1; i < period; ++i) {
        CALC_TRUERANGE();
        atr += truerange;
    }

and the second like this:

    for (i = period; i < size; ++i) {
        CALC_TRUERANGE();
        atr = atr * smth + truerange * per;
        ...

@rdbuf
Copy link
Author

rdbuf commented Mar 30, 2019

Done. By the way, what's your approach for quick microbenchmarking within this library?

@codeplea
Copy link
Member

You can remove your #undefs too.

Ok, I think you might have a serious bug. Imagine if high[4] is the local high point, and the period is 5. Would HP ever get set to high[4]? It seem that HP would skip straight to high[5] for the first bar. Right?

You might want to look at stoch.c as an example of maintaining the highest and lowest. This comes up in a lot of indicators. Also, the method used by stoch.c seem to benchmark faster (it pushed ce past 100 mfps for me, but I'm not sure if I introduced any new bugs or not).

Let me know your thoughts.

@codeplea
Copy link
Member

codeplea commented Mar 30, 2019

Done. By the way, what's your approach for quick microbenchmarking within this library?

I'm just doing make benchmark -B && benchmark ce.

Make sure to remove -std=c89 -pedantic before benchmarking too. It makes a large difference.

@rdbuf
Copy link
Author

rdbuf commented Mar 30, 2019

Yes, my bad regarding the maintainance of max/min. I’ll push an update once I’m back to my pc.

That’s interesting because I found my version consistently performing a few percent better. I was benchmarking inside a VM though and using an ad-hoc bench code, not benchmark.c.

What’s to benchmark.c, it has a dependency on ta-lib, and I haven’t still figured out how to obtain it.

@rdbuf
Copy link
Author

rdbuf commented Mar 30, 2019

...yet it must be pretty easy, right?

@codeplea
Copy link
Member

ta-lib is from https://www.ta-lib.org/hdr_dw.html

Benchmark will do comparisons between TI and ta-lib. We should probably update benchmark in the future so the ta-lib comparison is optional.

@codeplea codeplea mentioned this pull request Mar 30, 2019
@rdbuf
Copy link
Author

rdbuf commented Mar 30, 2019

I've spot another bug in my code. Gonna make a _ref implementation (#59)

@codeplea
Copy link
Member

I laid the ground work for streaming indicators and reference indicators. Check out #60

I think the reference indicators will be a big help. There are a lot of edge cases in optimized indicator code.

@codeplea codeplea mentioned this pull request Mar 31, 2019
@rdbuf rdbuf force-pushed the ce branch 2 times, most recently from 9f4462d to a2c366f Compare April 4, 2019 13:08
@rdbuf
Copy link
Author

rdbuf commented Apr 4, 2019

I've fixed everything I got wrong last time and implemented the new interfaces. Benchmarks show that there is potentially a room for improvement, so I will probably revise it later. These are available in the CI build log (thanks to #63).

@codeplea codeplea merged commit 6cf8fce into TulipCharts:0.9 May 27, 2019
@rdbuf rdbuf deleted the ce branch July 3, 2019 06:10
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.

None yet

2 participants