-
Notifications
You must be signed in to change notification settings - Fork 998
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
WIP add: Calculate fees on backtest #282
base: master
Are you sure you want to change the base?
Conversation
This commit updates backtesting to be executed in a different fork, it allows to start up several different executions independently, either by different tabs or even different pairs at the same time. In the future, this process could be persisted in database with the intention of running long backtestings of years of information on several dozen pairs. The exclusive locking had to be disabled so that each fork also has access to transact with the database. It would be interesting to move it to a Postgres, including a potential docker file
This change adds calculation of fees during backtesting increasing the accuracy of the test.
// Exit Details | ||
const exitPrice = signalObject.price; // Price during trade exit | ||
const exitValue = Number((tradedQuantity * exitPrice).toFixed(2)); // Price * Quantity | ||
const exitFee = (exitValue * feesPerTrade) / 100; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If those are fees per Trade, then why is it multiplied with the exit Value?
Shouldn't it be named relativeFees or something then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is my understanding that the fees are paid when we buy and then when we sell depending on each value, I took the info from: https://www.binance.com/en/support/faq/360033544231
I'm thinking that this code doesn't consider if for example the trade starts as a limit
order and ends as a market
one, simply applies a taker
fee to the whole process. I wonder if the UI backtesting would need to be updated to include those options
|
||
trades.profitabilityPercent = Number(((trades.profitableCount * 100) / trades.total).toFixed(2)); | ||
|
||
const summary = { | ||
sharpeRatio: sharpeRatio, | ||
averagePNLPercent: averagePNLPercent, | ||
netProfit: netProfit, | ||
initialCapital: initialCapital, | ||
netFees: cumulativeNetFees, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
netFees would imply there to also to be grossFees
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sorry, I'm not familiar with the financial term, am I missing more information to be shown in the screen? Thanks!
extended: true, | ||
parameterLimit: 50000 | ||
}) | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not a big fan of refactoring code if it doesn't have anything to do with the feature.
Please create another pull request for this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this might be the prettier
plugin
@@ -155,7 +155,7 @@ module.exports = { | |||
myDb.pragma('journal_mode = WAL'); | |||
|
|||
myDb.pragma('SYNCHRONOUS = 1;'); | |||
myDb.pragma('LOCKING_MODE = EXCLUSIVE;'); | |||
// myDb.pragma('LOCKING_MODE = EXCLUSIVE;'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I specified it on the commit message bit it got lost in the log :) It's basically cos sqlite3 is for some reason not allowing multiple connections to the database, one from the fork and from the main app. Probably worth locking the queries instead of the connection to the db. Happy to change to for a different approach if you have any ideas
This change adds calculation of fees during backtesting increasing the accuracy of the test.
It is built on top of the backtest, so if the other PR gets merge, I'll rebase this one with whatever changes are required.
This commit updates backtesting to be executed in a different fork, it allows to start up several different executions independently, either by different tabs or even different pairs at the same time.
In the future, this process could be persisted in database with the intention of running long backtestings of years of information on several dozen pairs.
The exclusive locking had to be disabled so that each fork also has access to transact with the database. It would be interesting to move it to a Postgres, including a potential docker file