Skip to content

feature: Implements SFX#8801

Closed
SaiRevanth25 wants to merge 9 commits into
QuantConnect:masterfrom
SaiRevanth25:SFX
Closed

feature: Implements SFX#8801
SaiRevanth25 wants to merge 9 commits into
QuantConnect:masterfrom
SaiRevanth25:SFX

Conversation

@SaiRevanth25

Copy link
Copy Markdown
Contributor

Description

Implements SFX

Related Issue

#8130

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>

@SaiRevanth25

Copy link
Copy Markdown
Contributor Author

hello @jaredbroad. Can you please approve the workflows. Thanks

@jaredbroad

Copy link
Copy Markdown
Member

Thanks @SaiRevanth25 ! Please share the code for how you created the test data, thanks!

@SaiRevanth25

SaiRevanth25 commented May 29, 2025

Copy link
Copy Markdown
Contributor Author

I've used the talipp reference data. the source code is here

To get the SFX column values i took the mean value of atr, stdDev and ma_stdDev

@jhonabreul jhonabreul left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nice, thank you! Sharing some minor comments

Comment thread Indicators/SmoothedForceIndex.cs Outdated
Comment thread Tests/Indicators/SmoothedForceIndexTests.cs Outdated
Comment thread Indicators/SmoothedForceIndex.cs Outdated
Comment thread Algorithm/QCAlgorithm.Indicators.cs Outdated
Comment thread Algorithm/QCAlgorithm.Indicators.cs Outdated
Comment thread Tests/Indicators/SmoothedForceIndexTests.cs
@SaiRevanth25 SaiRevanth25 requested a review from jhonabreul May 30, 2025 10:40

@jhonabreul jhonabreul left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Almost there! Some more minor coments below

Comment thread Indicators/SmoothedForceIndex.cs Outdated
Comment thread Tests/Indicators/SmoothedForceIndexTests.cs
@SaiRevanth25 SaiRevanth25 requested a review from jhonabreul May 30, 2025 17:31

@jhonabreul jhonabreul left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good. Thank you!

@Martin-Molinero Martin-Molinero left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hey! Thank you for the contribution
I think generically there's a misconception about what this indicator is, SmoothedForceIndex is something else (lean already has ForceIndex, which uses volume), for me SFX seems to be an indicator developed by Flux Charts, see https://es.tradingview.com/script/gdz74zT6-Flux-Charts-SFX-Algo-Premium/ (paid walled), which is just a collection of other indicators
Not sure how much value is there in adding this indicator really, but if we add it, we shouldn't call it SmoothedForceIndex which will be confusing, I also can't seem to find what SFX stand for though, see https://www.fluxcharts.com/articles/Flux-Charts/sfx-algo/introduction

/// <param name="selector">Selects a value from the BaseData to send into the indicator, if null defaults to casting the input value to a TradeBar</param>
[DocumentationAttribute(Indicators)]

public SmoothedForceIndex SFX(Symbol symbol, int atrPeriod, int stdDevPeriod, int stdDevSmoothingPeriod, Resolution? resolution = null, MovingAverageType maType = MovingAverageType.Simple, Func<IBaseData, TradeBar> selector = null)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Volume isn't required for SFX implementation? the base type should be the more generic BarIndicator instead of TradeBarIndicator
maType -> should be expanded to movingAverageType see other similar arguments in this file
2027 empty line should be removed

protected override decimal ComputeNextValue(TradeBar input)
{
AverageTrueRange.Update(input);
StandardDeviation.Update(new IndicatorDataPoint(input.EndTime, input.Close));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Minor but why not just call .Update(input) instead of creating the indicator data point

@Martin-Molinero

Copy link
Copy Markdown
Member

Closing for now due to #8801 (review), also SFX being a closed source indicator which has many more features which does of talipp implementation, not what is expected

@Martin-Molinero Martin-Molinero mentioned this pull request Jun 4, 2025
4 tasks
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.

4 participants