-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
fix cases where losing epoch get a huge negative loss value #10216
Conversation
I'm not sure it does if i'm honest. i'm using the following test cases to look at this calculation - some results are from the issue - some are artificial to see which value has which influence ... DRAWDOWN_MULT = 0.075
results = [
(30.785, 0.35 / 100),
(-221.333, 15.98 / 100),
(-315.876, 40.04 / 100),
(-459.923, 50.84 / 100),
(-581.544, 68.98 / 100),
(64.167, 5.31 / 100),
(65.167, 5.31 / 100),
(66.167, 5.31 / 100),
(-628.566, 74.53 / 100),
(602.105, 16.04 / 100),
(602.105, 26.04 / 100),
(602.105, 66.04 / 100),
(602.105, 120.04 / 100), # Drawdown after profit
(-494.568, 70.59 / 100),
(-251.262, 35.02 / 100),
(-429.174, 46.65 / 100),
(-759.062, 82.04 / 100),
]
for (total_profit, rel_drawdown) in results:
res = -1 * (total_profit * (1 - rel_drawdown * DRAWDOWN_MULT))
loss_value = total_profit * (1 - rel_drawdown * DRAWDOWN_MULT)
if (total_profit < 0) and (loss_value > 0):
pass
else:
loss_value = -1 * loss_value
print(f"{res=:>10.7g}\t\t{loss_value=:>10.7g}\t\t{total_profit=:.3f},\t {rel_drawdown=:.2%} {res == loss_value}") For explanation: Output:
The 2 don't deviate - not in ANY of the provided cases (not from the issue, nor in the artificial ones. To be perfectly honest, i don't think the base calculation of this works the way it is now (the sign is just a small problem here). |
Closing this in favor of #10221 - which revises the calculation completely. |
Summary
On some cases, you can have losing epoch (total profit< 0), but it can have a big negative loss value, which make the bot think it's the best epoch. This should fix such bug
Solve the issue: #10192
Quick changelog
What's new?