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

Implements new indicator ForceIndex #8155

Conversation

femtotrader
Copy link
Contributor

@femtotrader femtotrader commented Jul 5, 2024

Description

Implement new indicator - ForceIndex

Related Issue

Closes #8123

Motivation and Context

The ForceIndex indicator wasn't implemented

Requires Documentation Change

Doc for this indicator should be created but some docstring are written

How Has This Been Tested?

comparison with talipp computation of ForceIndex(20) of SPY candle data using generate_reference_data_from_talipp.py

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • Refactor (non-breaking change which improves implementation)
  • Performance (non-breaking change which improves performance. Please add associated performance test and results)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Non-functional change (xml comments/documentation/etc)

Checklist:

  • My code follows the code style of this project.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • My branch follows the naming convention bug-<issue#>-<description> or feature-<issue#>-<description>

@femtotrader
Copy link
Contributor Author

femtotrader commented Jul 5, 2024

I have no idea why AcceptsRenkoBarsAsInput test is failing.
All others tests are passing

@jaredbroad
Copy link
Member

Thanks, @femtotrader! For all the new PRs, can you please attach the source code of the comparison with Tulip, which generates the test data? We want to make sure all indicators have a third-party benchmark.

@femtotrader
Copy link
Contributor Author

Indicators are tested against talipp (Python) not tulipindicators (which is a great technical indicator lib also... but in C)

Source code of the comparison with talipp have ever been merged in this PR
#8148
see generate_reference_data_from_talipp.py

I don't have an other (second) comparison script for tulipindicators .

@femtotrader
Copy link
Contributor Author

Tulip indicators have a Python binding https://github.com/TulipCharts/tulipy which is marked as [NOT ACTIVELY MAINTAINED] ... so I don't know what to think about it.

This fork https://github.com/jesse-ai/tulipy (which have been used by Jesse bot) doesn't seems to be in a better state.

If comparison should be done against tulip indicators also we probably should rely on a C/C++ program for that purpose (which is an other story)

Copy link
Member

@Martin-Molinero Martin-Molinero left a comment

Choose a reason for hiding this comment

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

Nice! Looks good, sharing minor requests 👍

Indicators/ForceIndex.cs Outdated Show resolved Hide resolved
Indicators/ForceIndex.cs Outdated Show resolved Hide resolved
Indicators/ForceIndex.cs Outdated Show resolved Hide resolved
@femtotrader femtotrader changed the title Implement new indicator ForceIndex Implements new indicator ForceIndex Jul 11, 2024
Copy link
Member

@Martin-Molinero Martin-Molinero left a comment

Choose a reason for hiding this comment

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

Thank you @femtotrader! Almost there 👍 minor comment

Algorithm/QCAlgorithm.Indicators.cs Outdated Show resolved Hide resolved
@Martin-Molinero Martin-Molinero merged commit 14db9ca into QuantConnect:master Jul 16, 2024
6 of 7 checks passed
@femtotrader femtotrader deleted the feature-8123-new_indic_ForceIndex branch July 20, 2024 08:25
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.

Implement Force Index
3 participants